[PATCH] D48467: [X86] Recognize a splat of negate in isFNEG

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:36852
+      if (auto *C = cast_or_null<ConstantFP>(
+              getTargetConstantFromNode(Op1.getOperand(0))))
+        if (IsNeg(C))
----------------
eraman wrote:
> RKSimon wrote:
> > eraman wrote:
> > > RKSimon wrote:
> > > > Are there any circumstances that this isn't a ConstantFP? getTargetConstantFromNode peeks through bitcasts so don't you need to use dyn_cast_or_null?
> > > I have changed it to dyn_cast_or_null, but thinking about it I don't think that is needed.  First, the current code reads 
> > > 
> > > if (Op1.getOpcode() == X86ISD::VBROADCAST) {
> > >     if (auto *C = getTargetConstantFromNode(Op1.getOperand(0)))
> > >       if (isSignMask(cast<ConstantFP>(C)))
> > > 
> > > The x86 vbroadcast instruction broadcasts floating point values, so I think the cast<ConstantFP> is right. 
> > If you look at getTargetConstantFromNode the first thing it does is call peekThroughBitcasts so it can have any type. In fact we should probably be checking that the vector's element size is correct as well.
> > 
> > This is the kind of thing a fuzz test finds in 3 months time......
> I don't get the comment about vector's element size.  Which verctor's element size should be checked here?
Looking at this again, you could simplify a lot of this by using getTargetConstantBitsFromNode instead to extract all the bits for you, avoiding all the BROADCAST/BUILD_VECTOR special cases as getTargetConstantBitsFromNode  should do all of that already.


Repository:
  rL LLVM

https://reviews.llvm.org/D48467