[PATCH] D35049: LSR tunings for SystemZ, with some minor common code changes

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet requested changes to this revision.
qcolombet added a comment.
This revision now requires changes to proceed.

Hi Jonas,

Unless I am mistaken, I see three different changes here.
First batch is

>   - New TTI hook: LSRWithInstrQueries(), which defaults to false. Controls if LSR should do instruction-based addressing evaluations by calling isLegalAddressingMode() and isFoldableMemAccessOffset() with the Instruction pointers.
> - isLegalAddressingMode() gets a new optional Instruction* parameter (defaults to nullptr) used by LSR if Target returns true in LSRWithInstrQueries(). All target methods have been updated as well.
>   - In LSR / isAMCompletelyFolded(): Let target look at instructions if it returns true in LSRWithInstrQueries().

Second batch is

> - In LSR / isAddressUse(): handle address operands of memset, memmove and memcpy as address uses.

Third batch is

> - In LSR / RateFormula(): Don't add to ImmCost if the instructions are already checked. It only adds confusion when the results are otherwise equal. Call isFoldableMemAccessOffset() for any LSRUse::Address, not just loads / stores.

Could you please split the patch accordingly?

More comments inlined.

Cheers,
-Quentin



================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:1657
+    return true;
+  }
+
----------------
I feel that this code does not belong here.

Indeed, we have quite a few isAMCompletelyFolded overloaded functions, and I believe not all invocations would go through that specific instance.
Instead, I would have expected this to happen to the lower most version of the isAMCompletelyFolded version. The one that calls isLegalAddressingMode.


================
Comment at: test/CodeGen/SystemZ/loop-01.ll:243
+
+; Test that a memcpy loop does not get a lot of lays before each mvc (D12 and no index-reg).
+%0 = type { %1, %2* }
----------------
Could you run `opt -instnamer` on the IR?


https://reviews.llvm.org/D35049