[PATCH] D75404: [X86] Not track size of the boudaryalign fragment during the layout

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 1 18:46:09 PST 2020


MaskRay added a comment.

> Thanks for deleting the Size member variable. It can avoid some cache invalidation problems. LGTM, but please wait for another pair of eyes.



> Closed by commit rG2ac19feb1571 <https://reviews.llvm.org/rG2ac19feb1571960b8e1479a451b45ab56da7034e>: [X86] Not track size of the boudaryalign fragment during the layout (authored by skan). · Explain Why

For this case, it is likely that @reames will want to review the change, so I provided a qualified approval. From D71916 <https://reviews.llvm.org/D71916> "LGTM - How a Patch Is Accepted":

> If it is likely that others will want to review a recently-posted patch, especially if there might be objections, but no one else has done so yet, it is also polite to provide a qualified approval (e.g., "LGTM, but please wait for a couple of days in case others wish to review"). If approval is received very quickly, a patch author may also elect to wait before committing (and this is certainly considered polite for non-trivial patches). Especially given the global nature of our community, this waiting time should be at least 24 hours. Please also be mindful of weekends and major holidays.

The advice may apply to https://reviews.llvm.org/D75017 as well. If Phabricator is telling the truth, with all due respect, Annita has not ever committed to llvm-project and has not actively reviewed the assembly mitigations (there may be some internal review process but that is not visible to us). It was likely that someone would suggest some wording change, so it was not very appropriate to commit just 3 hours (BTW, 18:00~22:00 are not working hours in Pacific Time) after Annita approved the patch. Additionally, IIUC, we are not native English speakers😹 and it would be very helpful to get wording suggestions from others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75404





More information about the llvm-commits mailing list