[PATCH] Check for dynamic allocas and inline asm that clobbers sp before building selection dag (PR19012)

Reid Kleckner rnk at google.com
Tue Mar 4 18:00:00 PST 2014


  lgtm


================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:122-131
@@ +121,12 @@
+        if (const InlineAsm *IA = dyn_cast<InlineAsm>(CS.getCalledValue())) {
+          unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
+          std::vector<TargetLowering::AsmOperandInfo> Ops =
+            TLI->ParseConstraints(CS);
+          for (size_t I = 0, E = Ops.size(); I != E; ++I) {
+            TargetLowering::AsmOperandInfo &Op = Ops[I];
+            if (Op.Type == InlineAsm::isClobber) {
+              TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
+              std::pair<unsigned, const TargetRegisterClass*> PhysReg =
+                TLI->getRegForInlineAsmConstraint(Op.ConstraintCode,
+                                                  Op.ConstraintVT);
+              if (PhysReg.first == SP) {
----------------
This is painful, but I think it's correct.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:865
@@ -865,1 +864,3 @@
+        // If we clobbered the stack pointer, MFI should know about it.
         MachineFrameInfo *MFI = DAG.getMachineFunction().getFrameInfo();
+        assert(MFI->hasInlineAsmWithSPAdjust());
----------------
No asserts builds will warn that MFI is unused here.  I guess I'd fold it into the assert.

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:128
@@ +127,3 @@
+            if (Op.Type == InlineAsm::isClobber) {
+              TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
+              std::pair<unsigned, const TargetRegisterClass*> PhysReg =
----------------
Clobbers don't have SDValue operands, so SDValue() here is correct.  That's probably worth a comment.

================
Comment at: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:132
@@ +131,3 @@
+                                                  Op.ConstraintVT);
+              if (PhysReg.first == SP) {
+                MF->getFrameInfo()->setHasInlineAsmWithSPAdjust(true);
----------------
nit: This if doesn't need braces.


================
Comment at: test/CodeGen/X86/stack-align-memcpy.ll:32
@@ +31,2 @@
+; CHECK-NOT: rep;movsl
+}
----------------
Can you add a positive test that the 'rep movs' optimization fires when we remove the dynamic alloca or stack realignment?


http://llvm-reviews.chandlerc.com/D2954



More information about the llvm-commits mailing list