[PATCH] D45543: [globalisel] Add a combiner helpers for extending loads and use them in a pre-legalize combiner for AArch64

Daniel Sanders via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 23 00:46:26 PDT 2018


dsanders added a comment.
Herald added a reviewer: javed.absar.

In https://reviews.llvm.org/D45543#1066099, @aditya_nandakumar wrote:

> Maybe we could have all the tests in one file called prelegalize-combine-extloads.mir


I went with three files to match how we've been organizing the tests for the other passes but you raise a good point here. There's a good argument for the combiner being tested with one file per CombinerHelper::try*() function. I think that's probably a better organization.

In https://reviews.llvm.org/D45543#1071617, @aemerson wrote:

> I'm assuming this is in Target/AArch64 because other targets haven't been updated to use the new opcodes yet? We do eventually want to use these representations for every target though right?


That's right. As you say, we'll want to support it in every target that supports the extending loads (which is most if not all of the in-tree targets). However, until the new opcodes are legal for those targets, there's not much point in combining them only to revert them back to load+extend in the legalizer.

One other thing to mention is that we don't have a target-independent combiner in GlobalISel at the moment. Each target implements its own combiner(s) and makes use of code in CombinerHelper (where appropriate) to share code. I expect these combines to be used by multiple targets so I've put the bulk of the code in CombinerHelper but each target will need to add a pass and call to it.


Repository:
  rL LLVM

https://reviews.llvm.org/D45543





More information about the llvm-commits mailing list