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

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:1657
+    return true;
+  }
+
----------------
qcolombet wrote:
> 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.
The reason that I put it here, is because this is where LU is available. The check can't be done without LU (which has the Fixups), so if it's not placed here, the argument lists of the other versions must be changed, as well as the call sites of it and isLegalUse() (and possibly more?) to make the Fixups available. Is this what you have in mind, and if so should LU replace the other LU-arguments like MinOffset etc?



================
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* }
----------------
qcolombet wrote:
> Could you run `opt -instnamer` on the IR?
done.


https://reviews.llvm.org/D35049