[PATCH] D68398: [Alignment][NFC] Value::getPointerAlignment returns MaybeAlign
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 10 01:44:54 PDT 2019
gchatelet marked an inline comment as done.
gchatelet added inline comments.
================
Comment at: llvm/lib/Analysis/Loads.cpp:48
+ return false;
}
----------------
gchatelet wrote:
> jdoerfert wrote:
> > While I'm all for making the "getAlign" function explicit eventually, I think it would be good to keep it as is for this patch as there doesn't seem to be a reason to do this here.
> >
> > fwiw: I'm behind but eventually I was going to refactor this: D66618
> >
> Which function are you talking about exactly? I don't see a `getAlign` function.
@jdoerfert
Ha I suppose you're talking about `getBaseAlign`.
I was looking at D66618 and I believe the code will be easier to modify once it is moved to `llvm::Align` (I'm biased obviously).
Also the `getPointerAlignment` function has already been massaged in D67918 to prevent reuse of the `Align` variable and make logic more local rather than spanning the whole function.
I believe you'll have to rebase D66618 and you'll have conflicts there already.
On my side I'd need to move forward with the introduction of `llvm::Align`, I can make progress on other fronts but I'll have to update `Value` and `Load` eventually.
Please let me know what are your plans and schedule with D66618 so I can plan accordingly. Thx!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68398/new/
https://reviews.llvm.org/D68398
More information about the llvm-commits
mailing list