[llvm] [SelectionDAG] Fix and improve TargetLowering::SimplifySetCC (PR #87646)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 12 03:02:37 PDT 2024


================
@@ -189,7 +191,7 @@ define i1 @test_48_16_8(ptr %y) {
 ; CHECK-LE-LABEL: test_48_16_8:
 ; CHECK-LE:       @ %bb.0:
 ; CHECK-LE-NEXT:    ldrh r0, [r0, #1]
-; CHECK-LE-NEXT:    cmp r0, #0
+; CHECK-LE-NEXT:    lsls r0, r0, #8
----------------
bjope wrote:

> Have you looked into this any further?

See https://github.com/llvm/llvm-project/pull/87646/commits/5e9f9605b13c6b5fa7833c8805630ebcfb89ea32#r1552445909

After that I also made sure the test is run both with `-mtriple arm` and `-mtriple armv7` to show the difference in ARMTargetLowering::allowsMisalignedMemoryAccesses depending on the actual triple.
My new code is adhering to the "Fast" settings in ARMTargetLowering, while DAGCombiner::isLegalNarrowLdSt isn't. But maybe we want to do these transforms regardless if the unaligned accesses are considered as "Fast"? It felt wrong to me to make it that aggressive.

I'm also not sure why this code in DAGCombiner.cpp
```
  // Ensure that this isn't going to produce an unsupported memory access.
  if (ShAmt) {
    assert(ShAmt % 8 == 0 && "ShAmt is byte offset");
    const unsigned ByteShAmt = ShAmt / 8;
    const Align LDSTAlign = LDST->getAlign();
    const Align NarrowAlign = commonAlignment(LDSTAlign, ByteShAmt);
    if (!TLI.allowsMemoryAccess(*DAG.getContext(), DAG.getDataLayout(), MemVT,
                                LDST->getAddressSpace(), NarrowAlign,
                                LDST->getMemOperand()->getFlags()))
      return false;
  }
```
is skipping to consider the "Fast" result from allowsMemoryAccess that would be given if using the extra bool argument.

I kind of think the new behavior is more correct in some sense (taking the "Fast" part of the TLI hook into account). But then I guess we want to do that in DAGCombiner::isLegalNarrowLdSt as well (potentially getting even more diffs that looks like "regressions", but that in fact might be optimizations in case unaligned accesses are costly).

https://github.com/llvm/llvm-project/pull/87646


More information about the llvm-commits mailing list