[PATCH] D155470: [AArch64] LSLFast to fold onto base address by default

[Dave Green via Phabricator via llvm-commits]

dmgreen added reviewers: chill, fhahn, Allen.
dmgreen added a comment.
Herald added a subscriber: StephenFan.

I agree that it makes sense to do this more aggressively from looking at some of the optimisation manuals. (lsl-fast has taken the odd route of being originally added to represent when shifts into addressing operands were quick, but has started now to just mean that the add with shift is cheap). Some of the older optimization manual mention shifts of 2 being slower, is that something that we need to take into account? I'm not sure about other non-arm cores too. Presumably there was a reason why people originally believed that operands with shifts would be better as separate instructions.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:660
+bool AArch64DAGToDAGISel::isWorthFolding(SDValue V, bool FoldToBaseAddr = false) const {
+  bool AllowLSLFast = Subtarget->hasLSLFast() ? true : FoldToBaseAddr;
   // Trivial if we are optimizing for code size or if there is only
----------------
Subtarget->hasLSLFast() || FoldToBaseAddr


================
Comment at: llvm/test/CodeGen/AArch64/aarch64-fold-lslfast.ll:173-174
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK0: {{.*}}
+; CHECK3: {{.*}}
----------------
These CHECK prefixes can be removed now, if they are all expected to be the same.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-addr-mode-folding.ll:195
 ; CHECK: @test
 ; CHECK-NOT: , uxtw #2]
 define i32 @test(ptr %array, i8 zeroext %c, i32 %arg) {
----------------
Make sure to remove the old CHECK lines. It is probably worth updating the check lines in a separate patch, so that just the differences can be shown here.


================
Comment at: llvm/test/CodeGen/AArch64/arm64-fold-address.ll:62
 !4 = !{}
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK: {{.*}}
----------------
This looks like it hasn't been generated properly. It might mean the update script doesn't like the triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155470