[PATCH] D80745: [DAGCombiner] Add a command line option to guard ReduceLoadOpStoreWidth

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 28 15:59:14 PDT 2020


void added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15414
+  if (!EnableReduceLoadOpStoreWidth)
+    return SDValue();
+
----------------
efriedma wrote:
> There are two separate transforms here: ShrinkLoadReplaceStoreWithStore, which actually erases the load, and the isNarrowingProfitable transform, which just shrinks the store.  The performance implications might be different, so I think we want separate flags.
> 
> (We don't really want to encourage users to use these flags in production, but I think it's okay to add them to evaluate the performance impact.)
Would using this flag in `isNarrowingProfitable` (or conditionalizing the execution of same with the flag) better?

> (We don't really want to encourage users to use these flags in production, but I think it's okay to add them to evaluate the performance impact.)

Agreed. I think it's better to have a better heuristic to determine if narrowing is profitable.


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

https://reviews.llvm.org/D80745





More information about the llvm-commits mailing list