[PATCH] D10905: move DAGCombiner's allowableAlignment() helper function into the TLI

James Y Knight jyknight at google.com
Tue Jul 28 22:17:51 PDT 2015


jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

I do still think it might be ultimately better and cleaner to not involve DataLayout at all, and to have the target hook describe the complete rules on what alignments can be loaded/stored, since DataLayout is really about the system's defined ABI, which can legitimately differ in different environments, even though the hardware's behavior does not. And I expect it'd actually be easier to understand that way as well.

But this is definitely a solid improvement, and is a good change both if considered as a step towards the above and as the final state, so LGTM. Please update the description to match the new revision before committing.

[I'll also note that there's still a few other places left that call allowsMisalignedMemoryAccesses after this; I bet most of those could also be updated to call allowsMemoryAccess instead.]


http://reviews.llvm.org/D10905







More information about the llvm-commits mailing list