[llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.

[Eric Christopher]

Hi Petar,

It looks like it was fixed in gcc 4.5.4 according to that bug report and I'm loath
to turn off __builtin_unreachable for mips unconditionally because of a
bug that's known fixed in multiple released versions.

We could put it down as a known bad revision on the website for that architecture.

-eric

On Jul 24, 2012, at 2:48 PM, "Jovanovic, Petar" <petarj at mips.com> wrote:

> Hi Eric,
> 
> I have just updated the patch to include the comment for the change.
> 
> The issue is that code for some switch statements with __builtin_unreachable
> is not correctly generated. Some cases miss, and the code is wrong. The wrong
> code resulted in wrong values and eventually in null-pointer dereference and
> code crash. The issue has been found in Renderscript examples in Android that
> crashed due to incorrect llvm library code.
> 
> Regards,
> Petar
> ________________________________________
> From: Eric Christopher [echristo at apple.com]
> Sent: Tuesday, July 24, 2012 10:34 PM
> To: Jovanovic, Petar
> Cc: llvm-commits at cs.uiuc.edu; Benjamin Kramer
> Subject: Re: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
> 
> I'm not seeing any update in the code as to why you're working around it (which was Benjamin's request), and the comment that you made in your reply also doesn't state what the bug was other than "issue".
> 
> -eric
> 
> On Jul 24, 2012, at 11:46 AM, "Jovanovic, Petar" <petarj at mips.com> wrote:
> 
>> PTAL
>> 
>> -----Original Message-----
>> From: Petar Jovanovic [mailto:petar.jovanovic at rt-rk.com]
>> Sent: Thursday, April 12, 2012 3:15 AM
>> To: 'Benjamin Kramer'
>> Cc: 'llvm-commits at cs.uiuc.edu'
>> Subject: RE: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>> 
>> Any additional comments or concerns about this one?
>> 
>> Regards,
>> Petar
>> 
>> -----Original Message-----
>> From: Petar Jovanovic [mailto:petar.jovanovic at rt-rk.com]
>> Sent: Friday, March 23, 2012 7:15 PM
>> To: 'Benjamin Kramer'
>> Cc: 'llvm-commits at cs.uiuc.edu'
>> Subject: RE: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>> 
>> The issue was first found on the latest cross gcc from Mentor [gcc version 4.5.2 (Sourcery CodeBench Lite 2011.09-75)], and since __builtin_unreachable() is available as of gcc 4.5, it seemed to me that the scope is already narrow enough. I can also confirm that the bug can be reproduced with native gcc (GCC) 4.5.1 and gcc (GCC) 4.5.3 , and I am actually not aware of gcc version that supports __builtin_unreachable in which the bug is not present.
>> 
>> The bug was discovered three days ago in Android builds for MIPS. The function where this issue was discovered is:
>> unsigned TargetData::getAlignment(Type *Ty, bool abi_or_pref)
>> 
>> at lib\Target\TargetData.cpp.
>> 
>> Petar
>> 
>> -----Original Message-----
>> From: Benjamin Kramer [mailto:benny.kra at googlemail.com]
>> Sent: Friday, March 23, 2012 12:57 AM
>> To: Petar Jovanovic
>> Cc: llvm-commits at cs.uiuc.edu; Petar Jovanovic
>> Subject: Re: [llvm-commits] [PATCH] [MIPS] Avoid use of __builtin_unreachable() when compiling LLVM for MIPS.
>> 
>> On 23.03.2012, at 00:20, Petar Jovanovic <petar.jovanovic at rt-rk.com> wrote:
>> 
>>> There is a GCC bug which can be triggered inside of LLVM library. The -O2 opt
>>> does not work well with __builtin_unreachable(), so we are avoiding it inside
>>> of LLVM. The issue seems to be caused by 'freorder-blocks' optimization.
>> 
>> Patches working around compiler bugs are generally okay, but please
>> 
>> - Add a comment on why you're blacklisting MIPS here.
>> - Make the blacklist as narrow as possible, i.e. check for the
>> specific version of GCC that has this bug.
>> 
>> - Ben
>>> 
>>> Signed-off-by: Petar Jovanovic <petarj at mips.com>
>>> ---
>>> include/llvm/Support/Compiler.h |    2 ++
>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/include/llvm/Support/Compiler.h b/include/llvm/Support/Compiler.h
>>> index 9c5e7ec..b5a086a 100644
>>> --- a/include/llvm/Support/Compiler.h
>>> +++ b/include/llvm/Support/Compiler.h
>>> @@ -144,9 +144,11 @@
>>> // LLVM_BUILTIN_UNREACHABLE - On compilers which support it, expands
>>> // to an expression which states that it is undefined behavior for the
>>> // compiler to reach this point.  Otherwise is not defined.
>>> +#if !defined(__mips__)
>>> #if defined(__clang__) || (__GNUC__ > 4) \
>>> || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)
>>> # define LLVM_BUILTIN_UNREACHABLE __builtin_unreachable()
>>> #endif
>>> +#endif
>>> 
>>> #endif
>>> --
>>> 1.7.5.4
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> <disable-builtin-unreachable.diff>