[PATCH] [ARM] Align stack objects passed to functions

Renato Golin renato.golin at linaro.org
Fri Mar 6 08:26:25 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1236
@@ +1235,3 @@
+  unsigned AllocaSize = 0, AllocaAlign = 0;
+  if (TLI && TD && TLI->shouldAlignAllocaArgs(CI, AllocaSize, AllocaAlign)) {
+    assert(AllocaAlign != 0 && "shouldAlignAllocaArgs must set AllocaAlign");
----------------
john.brawn wrote:
> rengolin wrote:
> > I'm a bit averse of setting parameters for the computation inside a conditional and having to assert right after every call. I'd rather have (or reuse) some default size/alignment parameters from elsewhere.
> If an implementation of shouldAlignAllocaArgs were to not set AllocaAlign then that indicates that it's not doing what it should, so failing an assert is better than hiding that by silently using a default value.
I'm not against the assert, I'm against passing a reference for AllocaSize and AllocaAlign on a check call. The call is not explicit that it's setting the two values here, nor is it on TargetLowering's default implementation.

A more clear way would be to make a function whose only job is to set those variables and make it explicit on its name, like "setAlignAllocaArgs(CI, Size, Align)" and add "Size > 0" to the conditional. You can avoid the assert in that case, too.

Also, your check for TLI is redundant, since you already check for TD.

http://reviews.llvm.org/D7908

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list