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

Guozhi Wei via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 28 15:59:17 PDT 2020


Carrot added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15414
+  if (!EnableReduceLoadOpStoreWidth)
+    return SDValue();
+
----------------
void wrote:
> 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.
isNarrowingProfitable is also used to reduce load and other alu width, these don't stall following load.

I added a new flag to guard ShrinkLoadReplaceStoreWithStore.


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

https://reviews.llvm.org/D80745





More information about the llvm-commits mailing list