Remove check for single use in ShrinkDemandedConstant

[rampitec]

Summary

This removes check for single use from general ShrinkDemandedConstant
to the BE because of the AArch64 regression after D56289/rL350475.

After several hours of experiments I did not come up with a testcase
failing on any other targets if check is not performed.

Moreover, direct call to ShrinkDemandedConstant is not really needed
and superceed by SimplifyDemandedBits.

[fhahn]

Great thanks! IMO the caller of ShrinkDemandedConstant should make sure the demanded bits valid for all users, as the existing code in TargetLowering.cpp already should do.

[fhahn]

LGTM, thanks, but it would be good if someone more familiar with AMDGPU could also have a quick look.

[craig.topper]

Doesn't SimplifyDemandedBits already call ShrinkDemandedConstant for AND/OR/XOR? And without an override of targetShrinkDemandedConstant, ShrinkDemandedConstant only works on those 3 nodes. So is an explicit call to ShrinkDemandedConstant even needed here?

[arsenm]

Something seems wrong here. How/why is this valid on AArch64 in some situation? A test is really needed for whatever this regression is

[rampitec]

In D56406#1349488, @arsenm wrote:

Something seems wrong here. How/why is this valid on AArch64 in some situation? A test is really needed for whatever this regression is

AArch64 does not call this function directly. The common code does required checks. In fact if you look at rare uses of it in targets they are guarded by single use check. Anyway, I do not have these testcases.

[rampitec]

In D56406#1349338, @craig.topper wrote:

Doesn't SimplifyDemandedBits already call ShrinkDemandedConstant for AND/OR/XOR? And without an override of targetShrinkDemandedConstant, ShrinkDemandedConstant only works on those 3 nodes. So is an explicit call to ShrinkDemandedConstant even needed here?

Good point. Turns out I can simply drop it.

[rampitec]

Removed call to ShrinkDemandedConstant completely.

[arsenm]

LGTM

In D56406#1348948, @fhahn wrote:

LGTM, thanks, but it would be good if someone more familiar with AMDGPU could also have a quick look.

Can you commit a regression testcase for this?

[fhahn]

In D56406#1350562, @arsenm wrote:

LGTM

In D56406#1348948, @fhahn wrote:

LGTM, thanks, but it would be good if someone more familiar with AMDGPU could also have a quick look.

Can you commit a regression testcase for this?

Thanks for the patch :) I've committed rL350684 with a test case where we have an op with multiple users and a constant can be shrunk.

llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp

 /// bits set in that constant that are not demanded, then clear those bits and
 /// return true.
 bool TargetLowering::ShrinkDemandedConstant(SDValue Op, const APInt &Demanded,
                                             TargetLoweringOpt &TLO) const {
   SelectionDAG &DAG = TLO.DAG;
   SDLoc DL(Op);
   unsigned Opcode = Op.getOpcode();
 
-  if (!Op.hasOneUse())
-    return false;
-
   // Do target-specific constant optimization.
   if (targetShrinkDemandedConstant(Op, Demanded, TLO))
     return TLO.New.getNode();
 
   // FIXME: ISD::SELECT, ISD::SELECT_CC
   switch (Opcode) {
   default:
     break;

llvm/trunk/lib/Target/AMDGPU/SIISelLowering.cpp

SDValue SITargetLowering::performCvtF32UByteNCombine(SDNode *N,
   }
 
   APInt Demanded = APInt::getBitsSet(32, 8 * Offset, 8 * Offset + 8);
 
   KnownBits Known;
   TargetLowering::TargetLoweringOpt TLO(DAG, !DCI.isBeforeLegalize(),
                                         !DCI.isBeforeLegalizeOps());
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
-  if (TLI.ShrinkDemandedConstant(Src, Demanded, TLO) ||
-      TLI.SimplifyDemandedBits(Src, Demanded, Known, TLO)) {
+  if (TLI.SimplifyDemandedBits(Src, Demanded, Known, TLO)) {
     DCI.CommitTargetLoweringOpt(TLO);
   }
 
   return SDValue();
 }
 
 SDValue SITargetLowering::performClampCombine(SDNode *N,
                                               DAGCombinerInfo &DCI) const {