I actually think it is a problem with the optimizer like Kay first thought. -instcombine seems turning "((x and 6) shl 5) slt 32" into "(x and 6) slt 1". If the comparison were unsigned or the shl had a nsw flag, I think this would be okay. Since none of these is true, I don't think this transformation is correct. H. On Sat, Nov 16, 2013 at 1:41 AM, Mark Lacey <mark.lacey at apple.com> wrote:> > On Nov 15, 2013, at 3:42 PM, Kay Tiong Khoo <kkhoo at perfwizard.com> wrote: > > I've been diagnosing this bug: > http://llvm.org/bugs/show_bug.cgi?id=17827 > > Summary: I think the following program miscompiles at -O1 because the fact > that 'f0' is a signed 3-bit value is lost in the unoptimized LLVM IR. How > do we fix this? > > > I don’t have the C/C++ standards in front of me but IIRC whether a > char/short/int/long/long long bitfield is signed or unsigned is > implementation defined. You need to explicitly specify signed or unsigned > in order to have any guarantee of the signedness, e.g. signed int. > > > $ cat bitfield.c > /* %struct.S = type { i8, [3 x i8] } ??? */ > struct S { > int f0:3; > } a; > > int foo (int p) { > struct S c = a; > c.f0 = p & 6; > return c.f0 < 1; > } > > int main () { > return foo (4); > } > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/173f9ab3/attachment.html>
I need to read up on how nsw would make this different, but I see your point about the shift: %bf.result.shl = shl i8 %bf.value, 5 %bf.result.ashr = ashr i8 %bf.result.shl, 5 This should have splatted the sign bit across the upper 5 bits of the char, so the subsequent compare: %cmp = icmp slt i32 %bf.cast, 1 Can't be transformed to a check for 'equal to 0'. Thanks! On Fri, Nov 15, 2013 at 9:18 PM, Henrique Santos < henrique.nazare.santos at gmail.com> wrote:> I actually think it is a problem with the optimizer like Kay first > thought. -instcombine seems turning "((x and 6) shl 5) slt 32" into "(x and > 6) slt 1". If the comparison were unsigned or the shl had a nsw flag, I > think this would be okay. Since none of these is true, I don't think this > transformation is correct. > > H. > > > > On Sat, Nov 16, 2013 at 1:41 AM, Mark Lacey <mark.lacey at apple.com> wrote: > >> >> On Nov 15, 2013, at 3:42 PM, Kay Tiong Khoo <kkhoo at perfwizard.com> wrote: >> >> I've been diagnosing this bug: >> http://llvm.org/bugs/show_bug.cgi?id=17827 >> >> Summary: I think the following program miscompiles at -O1 because the >> fact that 'f0' is a signed 3-bit value is lost in the unoptimized LLVM IR. >> How do we fix this? >> >> >> I don’t have the C/C++ standards in front of me but IIRC whether a >> char/short/int/long/long long bitfield is signed or unsigned is >> implementation defined. You need to explicitly specify signed or unsigned >> in order to have any guarantee of the signedness, e.g. signed int. >> >> >> $ cat bitfield.c >> /* %struct.S = type { i8, [3 x i8] } ??? */ >> struct S { >> int f0:3; >> } a; >> >> int foo (int p) { >> struct S c = a; >> c.f0 = p & 6; >> return c.f0 < 1; >> } >> >> int main () { >> return foo (4); >> } >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/c1ed9272/attachment.html>
On Sat, Nov 16, 2013 at 3:39 PM, Kay Tiong Khoo <kkhoo at perfwizard.com>wrote:> I need to read up on how nsw would make this different, but I see your > point about the shift: >If the nsw flag is set, we can assume the sign bit isn't affected and apply the simplification. If the nuw flag is set, we can assume no bits are shifted out and apply the simplification if the comparison is unsigned. If no flag is set (and no flag is settable), then I don't think there's anything to be done.> %bf.result.shl = shl i8 %bf.value, 5 > %bf.result.ashr = ashr i8 %bf.result.shl, 5 > > This should have splatted the sign bit across the upper 5 bits of the > char, so the subsequent compare: > %cmp = icmp slt i32 %bf.cast, 1 > > Can't be transformed to a check for 'equal to 0'. > > Thanks! >H. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/283d630a/attachment.html>