[PATCH] D30708: [ARM] SMULW [T|B] DAG Combine

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 8 12:08:55 PST 2017


efriedma added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:1497
+  SelectionDAG &DAG = DCI.DAG;
+  ARMTargetLowering::TargetLoweringOpt TLO(DAG, true, true);
+
----------------
Explicitly passing "true" is probably a bad idea here.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:1499
+
+  if (TLO.SimplifyDemandedBits(N, OpIdx, Demanded, DCI))
+    return true;
----------------
You probably want to use TargetLowering::SimplifyDemandedBits rather than TargetLoweringOpt::SimplifyDemandedBits; the latter has weird handling for nodes with multiple uses.

I count eight nearly identical copies of this code, with just varying demanded bits; two here, combineBT in X86ISelLowering, two in XCoreTargetLowering::PerformDAGCombine, SITargetLowering::performCvtF32UByteNCombine, performTBISimplification in AArch64ISelLowering.cpp, simplifyI24 in AMDGPUISelLowering.cpp.  Please add a helper to TargetLowering for all of these to use.  (Don't worry about fixing the other copies in this patch.)


https://reviews.llvm.org/D30708





More information about the llvm-commits mailing list