[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