[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