[PATCH] D11160: Targets: commonize some stack realignment code

JF Bastien jfb at chromium.org
Mon Jul 20 20:39:02 PDT 2015


jfb added a comment.

@rnk / @hfinkel you may be interested in this comment about `needsStackRealignment`'s diagnostic. If you agree I'll submit another patch to remove the diagnostic.


================
Comment at: llvm/trunk/lib/CodeGen/TargetRegisterInfo.cpp:327
@@ +326,3 @@
+      return true;
+    DEBUG(dbgs() << "Can't realign function's stack: " << F->getName() << "\n");
+  }
----------------
I'm not sure this diagnostic makes sense: x86, ARM and MIPS implement `canRealignStack`, and a few places in the codebase check `needsStackRealignment` as one of a few conditions which are supposed to be commutable (in a chain of logical ors), or as a condition in a chain of logical ands where the call to `canRealignStack` was hoisted to be done unconditionally. When this happens, the code assumes that calling `needsStackRealignment` on invalid functions will just return false and the other checks will handle things. Adding `report_fatal_error` to `canRealignStack` breaks that assumption.

I wrote a patch (attached) which tries to fix all of these assumptions, but the code still fails on all the same tests. For example, ARM's `hasFP` check eventually ends up calling `canRealignStack` to decide whether `getFrameRegister` should return `FP` or `SP`. The basic situation seems to be sensible (if you can't realign the stack then your frame register is the stack pointer), but making this warning a hard-failure breaks the code.

Similar situations occur for other tests.

I think the right thing to do here is to remove this diagnostic: in some cases it makes sense, but only the (sometimes indirect) caller of `needsStackRealignment` knows whether that's the case or not. We could instead add an `isFatal` parameter, but that seems icky because the caller can be indirect (e.g. the previous `hasFP` example).

Turning this into `report_fatal_error` makes the following tests fail:
```
    LLVM :: CodeGen/ARM/2010-06-29-PartialRedefFastAlloc.ll
    LLVM :: CodeGen/ARM/2011-08-12-vmovqqqq-pseudo.ll
    LLVM :: CodeGen/ARM/2012-01-23-PostRA-LICM.ll
    LLVM :: CodeGen/ARM/struct_byval.ll
    LLVM :: CodeGen/ARM/struct_byval_arm_t1_t2.ll
    LLVM :: CodeGen/ARM/thumb1_return_sequence.ll
    LLVM :: CodeGen/ARM/vldm-sched-a9.ll
    LLVM :: CodeGen/ARM/vst3.ll
    LLVM :: CodeGen/Mips/msa/spill.ll
    LLVM :: CodeGen/Mips/no-odd-spreg-msa.ll
    LLVM :: CodeGen/Thumb/2009-08-20-ISelBug.ll
    LLVM :: CodeGen/Thumb/2011-05-11-DAGLegalizer.ll
    LLVM :: CodeGen/Thumb/large-stack.ll
    LLVM :: CodeGen/Thumb/long.ll
    LLVM :: CodeGen/Thumb2/crash.ll
    LLVM :: CodeGen/Thumb2/large-call.ll
    LLVM :: CodeGen/X86/avx-intel-ocl.ll
    LLVM :: CodeGen/X86/avx-load-store.ll
    LLVM :: CodeGen/X86/avx-vzeroupper.ll
    LLVM :: CodeGen/X86/avx512-intel-ocl.ll
    LLVM :: CodeGen/X86/dynamic-allocas-VLAs.ll
    LLVM :: CodeGen/X86/half.ll
    LLVM :: CodeGen/X86/musttail-fastcall.ll
    LLVM :: CodeGen/X86/pr18846.ll
    LLVM :: CodeGen/X86/preserve_allcc64.ll
    LLVM :: CodeGen/X86/stack-folding-fp-avx1.ll
    LLVM :: CodeGen/X86/stack-folding-int-avx1.ll
    LLVM :: CodeGen/X86/stack-folding-int-avx2.ll
    LLVM :: CodeGen/X86/stack-folding-xop.ll
    LLVM :: CodeGen/X86/unaligned-spill-folding.ll
    LLVM :: CodeGen/X86/vec_fp_to_int.ll
```

{F644622}


Repository:
  rL LLVM

http://reviews.llvm.org/D11160







More information about the llvm-commits mailing list