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

[Manman Ren]

On Feb 5, 2013, at 11:03 AM, Eric Christopher <echristo at gmail.com> wrote:

> 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.

That is true, with this patch, we are going from a compile time error with r172027 to a run time crash with warning, for the above case.
If the source code does not depend on the 32-byte alignment, we are going from a compile time error to a warning.

Before r172027, it is a run time crash without any warning.

Let me know what you think :]

Thanks,
Manman
 
> 
> 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/32060e05/attachment.html>