[PATCH] D25485: [x86] use 'neg' for negation of bool

[Sanjay Patel via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D25485#568598, @efriedma wrote:

> Can we do this in SelectionDAGLegalize::ExpandNode instead?  I suppose in theory some platform might prefer to lower this using shifts, but I can't think of any off the top of my head.


Would we still need to do the 'and' masking op though? In that case, there's probably no win?



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:16343
+  if (SrcVT.getScalarSizeInBits() == 1)
+    return DAG.getNode(ISD::SUB, DL, VT, DAG.getConstant(0, DL, VT), N0);
+
----------------
efriedma wrote:
> Given "SIGN_EXTEND_INREG(X, i1)", you can transform it to "-(X&1)". But you can't assume the input is zero-extended.  I think your patch misbehaves for the following testcase:
> 
> ```
> define void @_Z1fbi(i1 zeroext %a, i32 %b, i8* %p) local_unnamed_addr #0 {
> entry:
>   %conv = trunc i32 %b to i1
>   %or = or i1 %a, %conv
>   %s = sext i1 %or to i8
>   store i8 %s, i8* %p
>   ret void
> }
> ```
Ah, I misunderstood the meaning/guarantee of the source value type of ISD::SIGN_EXTEND_INREG. 
So for the given example, we get:
  orb   %dil, %sil
  shlb   $7, %sil
  sarb   $7, %sil
  movb   %sil, (%rdx)

But with this patch:
  orb   %dil, %sil
  negb   %sil
  movb   %sil, (%rdx)

And that's wrong if any of the higher bits of %b / %sil are set.


https://reviews.llvm.org/D25485