[PATCH] D115670: Correct behavior of Vector boolean-operations, implement vector operator-

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:2915
   case BO_EQ:
-    Result = (LHSValue == RHSValue);
+    Result = -(LHSValue == RHSValue);
     break;
----------------
craig.topper wrote:
> craig.topper wrote:
> > erichkeane wrote:
> > > efriedma wrote:
> > > > Using "operator=" to assign an int to an APInt is going to lead to weird/confusing results.  For example, I think this produces the wrong result if you have an int128_t vector.
> > > Can you clarify your comment? The result type of one of these compare-ops is either an i8 or an i32 I think, so I would expect it to 'just work', right?  
> > > 
> > > What problem of operator= on an APInt gives the wrong result?  I guess I've never seen/don't know how to repro what you mean.
> > > 
> > > I DID try to get a test wtih a 'compare op' on an __int128 vector, but that ends up asserting in GetSignedVectorType
> > Don't compare ops produce a result with the same number of bits as the input? If the compare is a vector of int128 the result should also be a vector of int128. APInt::operator= takes a uint64_t as input and will zero extend it to 128 bits. I think we want to sign extend here.
> > 
> > I assume the APInt for result already been given its bit width before this call?
> The easy fix is probably just to do the assignment like it was before and then do `Result.negate()`.
Yes, the APInt should already be the right size.  And you do seem correct about the width being the size of the elements, so you were right about that.

THanks for the tip on the .negate, I can just do that!  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115670/new/

https://reviews.llvm.org/D115670