[PATCH] D59729: [GVN] non-functional code movement
Jameson Nash via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 26 18:18:43 PDT 2019
vtjnash marked 2 inline comments as done.
vtjnash added inline comments.
================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:314
+ if (!GV || !GV->isConstant() || !GV->hasDefinitiveInitializer())
return -1;
----------------
reames wrote:
> Remove this line, not NFC
As far as I could tell, all later code paths are guarded by the full condition that I'm now also using here:
https://github.com/llvm-mirror/llvm/blob/7b016165658561a2ec5bdffea134fe50cc9bcf7a/lib/Analysis/ConstantFolding.cpp#L538
https://github.com/llvm-mirror/llvm/blob/7b016165658561a2ec5bdffea134fe50cc9bcf7a/lib/Analysis/ConstantFolding.cpp#L669
https://github.com/llvm-mirror/llvm/blob/7b016165658561a2ec5bdffea134fe50cc9bcf7a/lib/Analysis/ConstantFolding.cpp#L620
https://github.com/llvm-mirror/llvm/blob/7b016165658561a2ec5bdffea134fe50cc9bcf7a/lib/Analysis/ConstantFolding.cpp#L606
I thought it seemed odd to check only part of the condition early, while deferring the full check until later.
================
Comment at: lib/Transforms/Utils/VNCoercion.cpp:331
// offset applied as appropriate.
- Src =
- ConstantExpr::getBitCast(Src, Type::getInt8PtrTy(Src->getContext(), AS));
- Constant *OffsetCst =
- ConstantInt::get(Type::getInt64Ty(Src->getContext()), (unsigned)Offset);
- Src = ConstantExpr::getGetElementPtr(Type::getInt8Ty(Src->getContext()), Src,
- OffsetCst);
+ if (Offset) {
+ Src =
----------------
OTOH, this might not be strictly NFC, since, while not a semantic difference in IR, I thought it seemed like `ConstantFoldLoadFromConstPtr` may sometimes have difficultly looking through this BitCast.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59729/new/
https://reviews.llvm.org/D59729
More information about the llvm-commits
mailing list