[PATCH] D30483: [DAG] More aggressively Inline TokenFactors

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1594
+        //  Inline TFs used multiple times if we're dealing with a small enough set of nodes
+        if ((Op.hasOneUse()  || Ops.size() < 32) && !is_contained(TFs, Op.getNode())) {
           // Queue up for processing.
----------------
Are these lines too long?


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1594
+        //  Inline TFs used multiple times if we're dealing with a small enough set of nodes
+        if ((Op.hasOneUse()  || Ops.size() < 32) && !is_contained(TFs, Op.getNode())) {
           // Queue up for processing.
----------------
hfinkel wrote:
> Are these lines too long?
Please add a cl::opt instead of embedding 32 in the code.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1594
+        //  Inline TFs used multiple times if we're dealing with a small enough set of nodes
+        if ((Op.hasOneUse()  || Ops.size() < 32) && !is_contained(TFs, Op.getNode())) {
           // Queue up for processing.
----------------
hfinkel wrote:
> hfinkel wrote:
> > Are these lines too long?
> Please add a cl::opt instead of embedding 32 in the code.
Also, please add a comment that explains the limit here. The issue is that, if the TF operand is a large TF, then doing this will "duplicate" it (by folding it into other TFs) many times. In that light, it seems like Ops.size()*Ops.use_size() is really the relevant thing to bound.



================
Comment at: test/CodeGen/PowerPC/complex-return.ll:12
   %imag = getelementptr inbounds { ppc_fp128, ppc_fp128 }, { ppc_fp128, ppc_fp128 }* %x, i32 0, i32 1
-  store ppc_fp128 0xM400C0000000000000000000000000000, ppc_fp128* %real
-  store ppc_fp128 0xMC00547AE147AE1483CA47AE147AE147A, ppc_fp128* %imag
+  store ppc_fp128 0xM400C0000000033300000000888800001, ppc_fp128* %real
+  store ppc_fp128 0xMC00547AE147AE1483CA47AE147AE149A, ppc_fp128* %imag
----------------
Why is this changing?


https://reviews.llvm.org/D30483