[all-commits] [llvm/llvm-project] bc1f3e: [DAGCombiner] Pre-commit test case for ReduceLoadO...

[Björn Pettersson via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: bc1f3eb59333d32797db234c0edf4dc270469b0e
      https://github.com/llvm/llvm-project/commit/bc1f3eb59333d32797db234c0edf4dc270469b0e
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2024-12-11 (Wed, 11 Dec 2024)

  Changed paths:
    M llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    A llvm/test/CodeGen/ARM/dagcombine-ld-op-st.ll

  Log Message:
  -----------
  [DAGCombiner] Pre-commit test case for ReduceLoadOpStoreWidth. NFC

Adding test cases related to narrowing of load-op-store sequences.
ReduceLoadOpStoreWidth isn't careful enough, so it may end up
creating load/store operations that access memory outside the region
touched by the original load/store. Using ARM as a target for the
test cases to show what happens for both little-endian and big-endian.

This patch also adds a way to override the TLI.isNarrowingProfitable
check in DAGCombiner::ReduceLoadOpStoreWidth by using the option
-combiner-reduce-load-op-store-width-force-narrowing-profitable.
Idea is that it should be simpler to for example add lit tests
verifying that the code is correct for big-endian (which otherwise
is difficult since there are no in-tree big-endian targets that
is overriding TLI.isNarrowingProfitable).

This is a pre-commit for
  https://github.com/llvm/llvm-project/pull/119203


  Commit: 22780f808a6dba83bad9390f762095f263324df9
      https://github.com/llvm/llvm-project/commit/22780f808a6dba83bad9390f762095f263324df9
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2024-12-11 (Wed, 11 Dec 2024)

  Changed paths:
    M llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    M llvm/test/CodeGen/ARM/dagcombine-ld-op-st.ll
    M llvm/test/CodeGen/X86/store_op_load_fold.ll

  Log Message:
  -----------
  [DAGCombiner] Fix to avoid writing outside original store in ReduceLoadOpStoreWidth (#119203)

DAGCombiner::ReduceLoadOpStoreWidth could replace memory accesses
with more narrow loads/store, although sometimes the new load/store
would touch memory outside the original object. That seemed wrong
and this patch is simply avoiding doing the DAG combine in such
situations.

Also simplifying the expression used to align ShAmt down to a multiple
of NewBW. Subtracting (ShAmt % NewBW) should do the same thing as the
old more complicated expression.

Intention is to follow up with a patch that make more attempts, trying
to align the memory accesses at other offsets, allowing to trigger
the transform in more situations. The current strategy for deciding
size (NewBW) and offset (ShAmt) for the narrowed operations are a bit
ad-hoc, and not really considering big endian memory order in same
way as little endian.


Compare: https://github.com/llvm/llvm-project/compare/03019c687f00...22780f808a6d

To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications