[PATCH] D80924: [CostModel] Use MaybeAlign in getMemoryOpCost

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 4 07:38:11 PDT 2020


gchatelet added a comment.

In D80924#2073067 <https://reviews.llvm.org/D80924#2073067>, @samparker wrote:

> Looking again at the backends, it looks like a very involved task to migrate to Align - particularly because getMaskedMemoryOpCost still uses an unsigned value. So I'd still like to get this patch in for consistency, without having to make large changes to several of the backends (of which I'm not even sure what I'd need to do). As it stands, I assume there's some implicit casting going so these base implementations aren't even receiving an Align object in most cases.


Migrating to `Align`/`MaybeAlign` is indeed quite an involved task.
I've been working on it for almost a year now and that's why I opposed this patch ( to me it's like deconstructing what I've been fighting for :) )

`Align`/`MaybeAlign` are especially designed so there are no implicit casting. One have to call `.value()` and test for value availability in case of `MaybeAlign`.
I haven't had time to convert all the APIs yet but I intend to. In the meantime I'd appreciate if you could leave it as is.

If you feel strongly about it you can still submit this patch but this will be more work on my plate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80924





More information about the llvm-commits mailing list