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.
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.
Question 2: Why SignedOverflowBehavior is set to LangOptions::SOB_Undefined
by default?
Question 3: What about CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)
Because of nsw, and with Quentin patch (git-svn-id:
https://llvm.org/svn/llvm-project/llvm/trunk at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150810/219aa61c/attachment.html>
Jonathan Roelofs via llvm-dev
2015-Aug-11 02:49 UTC
[llvm-dev] [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 at 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
Jonathan Roelofs via llvm-dev
2015-Aug-11 15:03 UTC
[llvm-dev] [cfe-dev] NSW and ExtLdPromotion()
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 at 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