[llvm-dev] [cfe-dev] NSW and ExtLdPromotion()

[Jonathan Roelofs via llvm-dev]

On 8/10/15 10:40 PM, Lawrence wrote:
> Thanks for your prompt response, Jonathan, that's one of my suspicion
> too.
>
> However that means the only safe way to do a 32bit * 32bit is:  (cast
> to 64-bit)32bit * (cast to 64-bit) 32bit,   because mostly a 32-bit
> can't hold the result of 32bit * 32bit, is that so?

Depends on what you mean by "safe".  If that's: "well defined for all 
input values in the type's domain", then yes.  Alternatively, you can 
enable UBSan, and detect these kinds of issues at runtime.


FYI, the "overflow ==> UB" thing I mentioned only applies to signed 
integers:

   6.2.5 (9)

   A computation involving unsigned operands can never overflow, because
   a result that cannot be represented by the resulting unsigned integer
   type is reduced modulo the number that is one greater than the
   largest value that can be represented by the resulting type.


Jon

>
> Regards
>
> Lawrence Hu
>
> -----Original Message----- From: Jonathan Roelofs
> [mailto:jonathan at codesourcery.com] Sent: Monday, August 10, 2015 7:49
> PM To: Lawrence; cfe-dev at lists.llvm.org Cc: llvm-dev at lists.llvm.org
> Subject: Re: [cfe-dev] NSW and ExtLdPromotion()
>
>
>
> On 8/10/15 7:53 PM, Lawrence wrote:
>> Hi, All:
>>
>> I have a testcase which produced incorrect result, it’s caused by
>> the combination of nsw flag and ExtLdPromotion, I am leaning to say
>> Clang set nsw flag incorrectly, but please let me know if I was
>> wrong.
>
> nsw is set correctly in this testcase.  If the value of 'a' is large
> enough to cause the multiplication to overflow, then your program has
> undefined behavior.
>
>>
>> Here is the reduced testcase:
>>
>> long long foo(int *a)
>>
>> {
>>
>> long long c;
>>
>> c = *a * 1405;
>>
>> return c;
>>
>> }
>>
>> Clang emitted the following IR (It is done by EmitMUL() in
>> CGExprScalar.cpp, while
>> CGF.getLangOpts().getSignedOverflowBehavior()=LangOptions::SOB_Undefined
>>
>>
and CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)=false):
>>
>> ; Function Attrs: nounwind readonly
>>
>> define i64 @foo(i32* nocapture readonly %a) #0 {
>>
>> entry:
>>
>> %0 = load i32* %a, align 4, !tbaa !1
>>
>> %mul = mul nsw i32 %0, 1405
>>
>> %conv = sext i32 %mul to i64
>>
>> ret i64 %conv
>>
>> }
>>
>> Question 1: Is it reasonable to say “mul” is “No Signed Wrap” ?
>> *a *1405 could overflow though.
>
> The 'nsw' here means: "if the multiply overflows, the result is
> 'poison'", i.e. overflow leads to undefined behavior.
>
> http://llvm.org/docs/LangRef.html#poisonvalues
>
>>
>> Question 2: Why SignedOverflowBehavior is set to
>> LangOptions::SOB_Undefined by default?
>
> That's what the standard mandates.
>
>>
>> Question 3: What about
>> CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)
>
> What about it?
>
>>
>> Because of nsw, and with Quentin patch (git-svn-id:
>> https://llvm.org/svn/llvm-project/llvm/trunk@233753
>> 91177308-0d34-0410-b5e6-96231b3b80d8), ExtLdPromotion() in
>> CodeGenPrepare.cpp is able promote the sext to the following:
>>
>> define i64 @foo(i32* nocapture readonly %a) #0 {
>>
>> entry:
>>
>> %0 = load i32* %a, align 4, !tbaa !1
>>
>> %conv = sext i32 %0 to i64
>>
>> %mul = mul nsw i64 %conv, 1405
>>
>> ret i64 %mul
>>
>> }
>>
>> This promotion itself looks fine to me if nsw is true, and the
>> final code becomes:
>>
>> ldrsw    x8, [x0]
>>
>> movz    w9, #0x57d
>>
>> mul      x0, x8, x9
>>
>> ret
>>
>> The results is different from a 32-bit mul then sext, at least for
>> my testcase.
>>
>> Without nsw, ExtLdPromotion() didn’t change anything, and the
>> result is correct.
>>
>> Any thoughts would be helpful.
>>
>> Regards
>>
>> Lawrence Hu
>>
>>
>>
>> _______________________________________________ cfe-dev mailing
>> list cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded