Page MenuHomePhabricator

[Alignment][NFC] MachineMemOperand::getAlign/getBaseAlign

Authored by gchatelet on Mar 27 2020, 7:03 AM.



This is patch is part of a series to introduce an Alignment type.
See this thread for context:
See this patch for the introduction of the type:

Diff Detail

Event Timeline

gchatelet created this revision.Mar 27 2020, 7:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2020, 7:03 AM
courbet requested changes to this revision.Mar 27 2020, 7:16 AM
courbet added inline comments.

The outer Align looks unnecessary.


This is not an NFC. Should this be isAligned() ?



This revision now requires changes to proceed.Mar 27 2020, 7:16 AM
gchatelet marked 4 inline comments as done.Mar 27 2020, 8:01 AM
gchatelet added inline comments.

isAligned currently works the other way around, here is the definition

inline bool isAligned(Align Lhs, uint64_t SizeInBytes) {
  return SizeInBytes % Lhs.value() == 0;

Rewritten with isAligned, !(MMO->getAlignment()%16 gives isAligned(Align(16), MMO->getAlign().value()), this is not very readable.

Now !(A % 16) means that A should be a multiple of 16.
Since A is a power of two by definition it is a multiple of 16 iff it is >=16.

So although it is not a straightforward it is NFC.


same here

courbet accepted this revision.Mar 27 2020, 8:16 AM

Thanks for the explanation.

This revision is now accepted and ready to land.Mar 27 2020, 8:16 AM
gchatelet marked 3 inline comments as done.Mar 27 2020, 8:47 AM
gchatelet updated this revision to Diff 253135.Mar 27 2020, 8:47 AM
  • Remove redundant Align
This revision was automatically updated to reflect the committed changes.