[PATCH] D47103: Implement strip.invariant.group
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue May 22 20:58:11 PDT 2018
rjmccall added a comment.
The changes to Clang generally seem reasonable, but I think you should split them into a separate commit from the commit that adds the intrinsic itself.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3858
+ }
+ }
+
----------------
Please add a comment explaining why this is necessary. (I'm actually not sure why it is, because surely the invariant groups we generate don't contain assumptions about memory from fields, right?)
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1626
+ Src = Builder.CreateStripInvariantGroup(Src);
+ }
+
----------------
Again, comment.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1767
+ if (CGF.CGM.getCodeGenOpts().StrictVTablePointers)
+ return Builder.CreateLaunderInvariantGroup(IntToPtr);
+
----------------
I think assigning to IntToPtr would be clearer here. You're changing the value returned, not introducing a reason to early-exit.
And a comment, please, on all of these cases.
Repository:
rL LLVM
https://reviews.llvm.org/D47103
More information about the cfe-commits
mailing list