[llvm] r174336 - [Stack Alignment] emit warning instead of a hard error

[Eric Christopher]

Well, I think we've gone from a compile time error for code like this:

void foo () {
  my_32byte_align_requested_struct a;

   <insert inline asm that depends upon 32-byte alignment>

}

to a run time crash.

It may be pretty uncommon, but perhaps some other examples for the type of
code you're seeing would be good? I remember some code with 4 byte
alignment depending upon 16-byte alignment at one point being difficult to
track down due to this...

-eric


On Tue, Feb 5, 2013 at 10:53 AM, Jim Grosbach <grosbach at apple.com> wrote:

> Thanks for clarifying.
>
> With that in mind, I'm not personally opposed to demoting this to a
> warning anyway, but I will grant that it does somewhat weaken the case for
> it.
>
> Eric, what do you think?
>
> -Jim
>
> On Feb 5, 2013, at 10:43 AM, Manman Ren <mren at apple.com> wrote:
>
>
> Sorry for the confusion :]
> We are not performing the frontend analysis to check for whether the low
> bits are assumed to be zeros.
> I was stating that we should emit a hard error if the low bits are assumed
> to be zeros in the source code, and a warning if the low bits are not
> assumed to be zeros.
> But since we don't currently perform the frontend analysis, to make sure
> existing codes that compile with previous version can still build, we emit
> a warning.
>
> Hope that clears things out.
>
> Manman
>
> On Feb 5, 2013, at 10:36 AM, Jim Grosbach <grosbach at apple.com> wrote:
>
> Hi Manman,
>
> Per discussion in rdar://13127907, we should emit a hard error only if
> people write code where the requested alignment is larger than achievable
> and assumes the low bits are zeros. A warning should be good enough when
> we are not sure if the source code assumes the low bits are zeros.
>
>
> I'm a bit confused. This implies that we're doing the frontend analysis to
> check for that condition and issue a hard error for it. The below is saying
> that's not actually the case. Can you elaborate a bit on what exactly is
> happening?
>
>
> -Jim
>
> On Feb 5, 2013, at 10:18 AM, Manman Ren <mren at apple.com> wrote:
>
>
> We currently do not analyze the source code to check the usage of the low
> bits.
>
> In the backend, the alignment is already clamped to the correct value, so
> the backend optimizations will not treat those low bits as zero.
>
> -Manman
>
> On Feb 4, 2013, at 6:08 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> Also I could be missing it but I couldn't spot the code that checks for
> the usage of the bits in not aligning/warning/erroring? Quick pointer?
>
> Thanks!
>
> -eric
>
>
> On Mon, Feb 4, 2013 at 5:41 PM, Eric Christopher <echristo at gmail.com>wrote:
>
>>
>>
>>
>> On Mon, Feb 4, 2013 at 5:35 PM, Manman Ren <mren at apple.com> wrote:
>>
>>>
>>> Yes, there are related discussions in r169197 and "[PATCH] Stack
>>> Alignment: clamp the alignment of stack objects in MachineFrameInfo".
>>>
>>> But people can use a 32-byte alignment attribute on a machine which only
>>> supports 16-byte stack alignment.
>>> If the source code further assumes the low bits are zeros, they will get
>>> wrong result.
>>> But if not, a hard error is too much and it will make existing code
>>> which can compile with earlier version failed to build with this patch.
>>>
>>
>> And to use the other side of the argument that won last time :)
>>
>> But this means that if people aren't looking at the warning or hard
>> erroring on warnings then we're going to emit bad code instead of making it
>> an error.
>>
>> -eric
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130205/19dfe385/attachment.html>