[PATCH] D51751: Merge clang's isRepeatedBytePattern with LLVM's isBytewiseValue
Matthias Braun via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 7 13:52:01 PDT 2018
MatzeB added a comment.
LGTM once Eli approves.
================
Comment at: lib/Analysis/ValueTracking.cpp:3046
Value *llvm::isBytewiseValue(Value *V) {
+ LLVMContext &Ctx = V->getContext();
+
----------------
You could move this down, closer to its first user.
================
Comment at: lib/Analysis/ValueTracking.cpp:3059-3064
+ // Conceptually, we could handle things like:
+ // %a = zext i8 %X to i16
+ // %b = shl i16 %a, 8
+ // %c = or i16 %a, %b
+ // but until there is an example that actually needs this, it doesn't seem
+ // worth worrying about.
----------------
Indeed an interesting discussion whether we should call into something like InstructionSimplify here. I agree with you that we'd rather see this handled in a separate step by whoever calls `isByteWiseValue()`. That way we can keep this function simple and only handle the patterns that are already simplified.
================
Comment at: lib/Analysis/ValueTracking.cpp:3129
- // Conceptually, we could handle things like:
- // %a = zext i8 %X to i16
- // %b = shl i16 %a, 8
- // %c = or i16 %a, %b
- // but until there is an example that actually needs this, it doesn't seem
- // worth worrying about.
+ // BlockAddress, ConstantExpr, and everything else is scary.
return nullptr;
----------------
not sure how valuable this comment is :)
Repository:
rL LLVM
https://reviews.llvm.org/D51751
More information about the llvm-commits
mailing list