[PATCH] D61887: [SelectionDAG] computeKnownBits - support constant pool values from target

Wed May 22 01:30:20 PDT 2019

RKSimon marked an inline comment as done.
RKSimon added a comment.

In D61887#1510934 <https://reviews.llvm.org/D61887#1510934>, @spatel wrote:

> Do we get any changes with only the computeKnownBits part of the patch (ie, without the demanded bits part of the patch)?

The demanded bits part is necessary to stop an infinite loop (finds constant in load, creates new constant that gets lowered to a constant pool load, finds that constant in load, ....).

> It seems ok, but I'm wondering if we can reduce risk and the test diffs by splitting those changes up.

They're dependent on each other - SimplifyDemandedBits has no way to know when a node is a constant already - we already have to do something similar for BUILD_VECTOR of constants.

I can pull out the DAGCombiner.cpp change as a pre-commit - but I don't think it will affect any codegen without the rest of the patch.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5777

   auto *CNode = dyn_cast<ConstantPoolSDNode>(Ptr);
   if (!CNode || CNode->isMachineConstantPoolEntry() || CNode->getOffset() != 0)
----------------
craig.topper wrote:
> Is there an implicit assumption here that constant pool loads won't be a SEXTLOAD/ZEXTLOAD/EXTLOAD? If they were we'd get a mismatch of bitwidth right?
I think this can return a extension load constant - it's up to the caller to check the LoadSDNode - compute known bits explicitly only uses constants of the same bitwidth.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61887