[PATCH] D121744: [SCCP] Update ValueLatticeElement blockaddresses when removing unreachable BasicBlocks
Nick Desaulniers via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 15 15:08:11 PDT 2022
nickdesaulniers added inline comments.
================
Comment at: llvm/lib/Transforms/Scalar/SCCP.cpp:528-531
+ LLVMContext &Context = BB->getParent()->getContext();
+ ConstantInt *CI = ConstantInt::get(Type::getInt32Ty(Context), 1);
+ Constant *C =
+ ConstantExpr::getIntToPtr(CI, Type::getInt8PtrTy(Context));
----------------
There's probably some code somewhere in `llvm::SimplifyInstruction` that already does this transform (as observed by `SimplifyCFG` in https://github.com/llvm/llvm-project/issues/54328#issuecomment-1067229660). maybe that can be reused here?
Since the behavior is surprising to users (https://github.com/llvm/llvm-project/issues/54328) maybe we could consolidate such transforms with a comment about why this is done, for historical context?
================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:424-426
+ if (New->getType()->isPointerTy() && Old->getType()->isPointerTy() &&
+ New->getType() != Old->getType())
+ New = ConstantExpr::getBitCast(New, Old->getType());
----------------
This added bitcast is a bit curious, I'd like to clean it up, but I'm not sure yet how best to do so.
Basically, we know we want to replace `i8* blockaddress(@fn, %bb)` with `i8* inttoptr (i32 1 to i8*)`, but the `User` of the BA and entry in `ValueState` is `%1 = bitcast i8* blockaddress(@fn, %bb) to i64*`.
Maybe I should be doing an operand replacement? As in take `Old` and replace its operand with `New`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121744/new/
https://reviews.llvm.org/D121744
More information about the llvm-commits
mailing list