[PATCH] D11160: Targets: commonize some stack realignment code
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
More information about the llvm-commits