]> granicus.if.org Git - llvm/commitdiff
Don't crash when we see unallocatable registers in clobbers
authorGeorge Burgess IV <george.burgess.iv@gmail.com>
Mon, 23 Oct 2017 20:46:36 +0000 (20:46 +0000)
committerGeorge Burgess IV <george.burgess.iv@gmail.com>
Mon, 23 Oct 2017 20:46:36 +0000 (20:46 +0000)
This fixes a bug where we'd crash given code like the test-case from
https://bugs.llvm.org/show_bug.cgi?id=30792 . Instead, we let the
offending clobber silently slide through.

This doesn't fully fix said bug, since the assembler will still complain
the moment it sees a crypto/fp/vector op, and we still don't diagnose
calls that require vector regs.

Differential Revision: https://reviews.llvm.org/D39030

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@316374 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
lib/CodeGen/SelectionDAG/LegalizeTypes.h
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
test/CodeGen/AArch64/no-fp-asm-clobbers-crash.ll [new file with mode: 0644]

index bbee69fb656170f5cbaa92ac49de39139584a6fa..ff49134f7b997722077fd180a68080b8430779e0 100644 (file)
@@ -971,7 +971,9 @@ getStrictFPOpcodeAction(const TargetLowering &TLI, unsigned Opcode, EVT VT) {
 void SelectionDAGLegalize::LegalizeOp(SDNode *Node) {
   DEBUG(dbgs() << "\nLegalizing: "; Node->dump(&DAG));
 
-  if (Node->getOpcode() == ISD::TargetConstant) // Allow illegal target nodes.
+  // Allow illegal target nodes and illegal registers.
+  if (Node->getOpcode() == ISD::TargetConstant ||
+      Node->getOpcode() == ISD::Register)
     return;
 
 #ifndef NDEBUG
@@ -985,7 +987,8 @@ void SelectionDAGLegalize::LegalizeOp(SDNode *Node) {
     assert((TLI.getTypeAction(*DAG.getContext(), Op.getValueType()) ==
               TargetLowering::TypeLegal ||
             TLI.isTypeLegal(Op.getValueType()) ||
-            Op.getOpcode() == ISD::TargetConstant) &&
+            Op.getOpcode() == ISD::TargetConstant ||
+            Op.getOpcode() == ISD::Register) &&
             "Unexpected illegal type!");
 #endif
 
index c46d1b04804c9cbb7fbc1014cbc5d39f2edf6277..094afe2830b8ef967b6191bbe52d5da26d141993 100644 (file)
@@ -89,7 +89,8 @@ private:
 
   /// Pretend all of this node's results are legal.
   bool IgnoreNodeResults(SDNode *N) const {
-    return N->getOpcode() == ISD::TargetConstant;
+    return N->getOpcode() == ISD::TargetConstant ||
+           N->getOpcode() == ISD::Register;
   }
 
   /// For integer nodes that are below legal width, this map indicates what
@@ -400,18 +401,22 @@ private:
   /// Given an operand Op of Float type, returns the integer if the Op is not
   /// supported in target HW and converted to the integer.
   /// The integer contains exactly the same bits as Op - only the type changed.
-  /// For example, if Op is an f32 which was softened to an i32, then this method
-  /// returns an i32, the bits of which coincide with those of Op.
+  /// For example, if Op is an f32 which was softened to an i32, then this
+  /// method returns an i32, the bits of which coincide with those of Op.
   /// If the Op can be efficiently supported in target HW or the operand must
   /// stay in a register, the Op is not converted to an integer.
   /// In that case, the given op is returned.
   SDValue GetSoftenedFloat(SDValue Op) {
-    SDValue &SoftenedOp = SoftenedFloats[Op];
-    if (!SoftenedOp.getNode() &&
-        isSimpleLegalType(Op.getValueType()))
+    auto Iter = SoftenedFloats.find(Op);
+    if (Iter == SoftenedFloats.end()) {
+      assert(isSimpleLegalType(Op.getValueType()) &&
+             "Operand wasn't converted to integer?");
       return Op;
+    }
+
+    SDValue &SoftenedOp = Iter->second;
+    assert(SoftenedOp.getNode() && "Unconverted op in SoftenedFloats?");
     RemapValue(SoftenedOp);
-    assert(SoftenedOp.getNode() && "Operand wasn't converted to integer?");
     return SoftenedOp;
   }
   void SetSoftenedFloat(SDValue Op, SDValue Result);
index 75641e05648eaee8f9dea069f8cb73bf280dc1c4..41d352ded377e845ec09711071176b36f4164e25 100644 (file)
@@ -935,7 +935,23 @@ void RegsForValue::AddInlineAsmOperands(unsigned Code, bool HasMatching,
   SDValue Res = DAG.getTargetConstant(Flag, dl, MVT::i32);
   Ops.push_back(Res);
 
-  unsigned SP = TLI.getStackPointerRegisterToSaveRestore();
+  if (Code == InlineAsm::Kind_Clobber) {
+    // Clobbers should always have a 1:1 mapping with registers, and may
+    // reference registers that have illegal (e.g. vector) types. Hence, we
+    // shouldn't try to apply any sort of splitting logic to them.
+    assert(Regs.size() == RegVTs.size() && Regs.size() == ValueVTs.size() &&
+           "No 1:1 mapping from clobbers to regs?");
+    unsigned SP = TLI.getStackPointerRegisterToSaveRestore();
+    for (unsigned I = 0, E = ValueVTs.size(); I != E; ++I) {
+      Ops.push_back(DAG.getRegister(Regs[I], RegVTs[I]));
+      assert(
+          (Regs[I] != SP ||
+           DAG.getMachineFunction().getFrameInfo().hasOpaqueSPAdjustment()) &&
+          "If we clobbered the stack pointer, MFI should know about it.");
+    }
+    return;
+  }
+
   for (unsigned Value = 0, Reg = 0, e = ValueVTs.size(); Value != e; ++Value) {
     unsigned NumRegs = TLI.getNumRegisters(*DAG.getContext(), ValueVTs[Value]);
     MVT RegisterVT = RegVTs[Value];
@@ -943,11 +959,6 @@ void RegsForValue::AddInlineAsmOperands(unsigned Code, bool HasMatching,
       assert(Reg < Regs.size() && "Mismatch in # registers expected");
       unsigned TheReg = Regs[Reg++];
       Ops.push_back(DAG.getRegister(TheReg, RegisterVT));
-
-      if (TheReg == SP && Code == InlineAsm::Kind_Clobber) {
-        // If we clobbered the stack pointer, MFI should know about it.
-        assert(DAG.getMachineFunction().getFrameInfo().hasOpaqueSPAdjustment());
-      }
     }
   }
 }
diff --git a/test/CodeGen/AArch64/no-fp-asm-clobbers-crash.ll b/test/CodeGen/AArch64/no-fp-asm-clobbers-crash.ll
new file mode 100644 (file)
index 0000000..5cd8dc5
--- /dev/null
@@ -0,0 +1,18 @@
+; RUN: llc < %s | FileCheck %s
+;
+; Be sure that we ignore clobbers of unallocatable registers, rather than
+; crashing.
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64"
+
+; CHECK-LABEL: foo:
+; CHECK: ret
+define void @foo() #0 {
+entry:
+  call void asm sideeffect "", "~{v0}"()
+  call void asm sideeffect "", "~{s0}"()
+  ret void
+}
+
+attributes #0 = { nounwind "target-features"="-crypto,-fp-armv8,-neon" }