[PATCH] D117356: InstructionCombining: avoid eliding mismatched alloc/free pairs

[Augie Fackler via Phabricator via llvm-commits]

durin42 marked 4 inline comments as done.
durin42 added a comment.

I'll poke the enum later today or first thing tomorrow. I got stuck in a tarpit of being sad about the `arc` command line tool today.



================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2666
         if (isFreeCall(I, &TLI)) {
+          if (getAllocationFamily(I, &TLI) != Family)
+            return false;
----------------
reames wrote:
> With the semantics you're proposing, having two unannotated alloc/free be removed seems inconsistent.  
> 
> Also, use compound if clause.
I think we can't fuse the conditions, because if we see a `free()` call on a `new`ed pointer we should give up on the instruction combining, not allow it to possibly proceed on other methods, right?

Agreed on the unannotated functions meshing being inconsistent. Today that can't happen, but I guess when we move this to supporting attributes it could. Should the logic be something more like:

if (isFreeCall(...)) {
  auto FreeFamily = getAllocationFamily(...);
  if (FreeFamily == None || FreeFamily != Family) 
    return false;
  ...
}

or is there more to it than that that I'm missing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117356/new/

https://reviews.llvm.org/D117356