[PATCH] D37933: Prevent InstCombine from miscompiling `operator delete`
Richard Smith - zygoloid via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 15 19:11:17 PST 2020
rsmith added a comment.
In D37933#872815 <https://reviews.llvm.org/D37933#872815>, @davide wrote:
> So, from what I can see no transformation made for free() is valid in case of `operator delete`.
> What do you think instead of introducing a predicate `isCallToOperatorDelete()` and bail out at the beginning of `visitFree()` ?
Sounds fine (but we should also check whether the call is a `nobuiltin` call to `free`, which should also not be optimized).
================
Comment at: lib/Analysis/MemoryBuiltins.cpp:351
+/// hasSideEffectsFreeCall - Returns true if the call has side effects.
+bool llvm::hasSideEffectsFreeCall(const CallInst *CI,
----------------
davide wrote:
> rsmith wrote:
> > This is a really confusing name. It sounds like it's detecting whether a call is free of side-effects, whereas it's *actually* detecting whether the call is to a `free`-like function that has side-effects.
> Not really happy about the name either, have a better proposal?
Maybe `freeCallHasSideEffects`? (Or, if we revert the sense of this as suggested below, `freeCallHasNoSideEffects`.)
================
Comment at: lib/Analysis/MemoryBuiltins.cpp:352-353
+/// hasSideEffectsFreeCall - Returns true if the call has side effects.
+bool llvm::hasSideEffectsFreeCall(const CallInst *CI,
+ const TargetLibraryInfo &TLI) {
+ Function *Callee = CI->getCalledFunction();
----------------
This should presumably also be checking whether the call is `nobuiltin`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D37933/new/
https://reviews.llvm.org/D37933
More information about the llvm-commits
mailing list