[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