[SimplifyLibcalls] Realloc(null, N) -> Malloc(N)
ClosedPublic

Authored by xbolva00 on Sun, Apr 8, 7:15 AM.

Details

Diff Detail

Repository
rL LLVM
xbolva00 created this revision.Sun, Apr 8, 7:15 AM
xbolva00 retitled this revision from [InstCombine] Fold malloc + realloc to [SimplifyLibcalls] Fold malloc + realloc.
xbolva00 added subscribers: efriedma, lebedev.ri.EditedWed, Apr 11, 9:15 PM

I would like to hear your feedback on these three changes

  1. malloc(N) + realloc(ptr, N + X) -> fold to -> malloc(N + X), remove realloc
  2. malloc(N) + realloc(ptr, N - X) -> fold to -> malloc(N), remove realloc
  3. realloc(NULL, N) -> malloc(N)

@efriedma @lebedev.ri @spatel and others...

Edit: I think only 3. should stay in this patch..

lib/Transforms/Utils/SimplifyLibCalls.cpp
847 ↗(On Diff #141545)

This is somehow problematic, if I remove this check, this code will crash on 32bit.

define i8* @foo(i32 %n) #0 {

%call = call noalias i8* @malloc(i32 %n) #2
%add = add nsw i32 %n, 2
%call1 = call i8* @realloc(i8* %call, i32 %add) #2
ret i8* %call1

}

Any ideas why, @spatel @efriedma ?

xbolva00 updated this revision to Diff 142122.Thu, Apr 12, 12:21 AM
xbolva00 edited the summary of this revision. (Show Details)
xbolva00 retitled this revision from [SimplifyLibcalls] Fold malloc + realloc to [SimplifyLibcalls] Realloc(null, N) -> Malloc(N).
xbolva00 updated this revision to Diff 142131.Thu, Apr 12, 1:54 AM
efriedma added inline comments.Thu, Apr 12, 12:27 PM
lib/Transforms/Utils/BuildLibCalls.cpp
1059 ↗(On Diff #142131)

Why are the implementations of emitMalloc and emitCalloc different? (I'm specifically concerned about missing call to inferLibFuncAttributes.)

test/Transforms/InstCombine/realloc.ll
13 ↗(On Diff #142131)

This doesn't look like it's what you meant to test.

xbolva00 added inline comments.Thu, Apr 12, 12:35 PM
test/Transforms/InstCombine/realloc.ll
13 ↗(On Diff #142131)

oh yes, I will rework test soon.

xbolva00 updated this revision to Diff 142247.Thu, Apr 12, 1:19 PM
xbolva00 added inline comments.
test/Transforms/InstCombine/realloc.ll
13 ↗(On Diff #142131)

Done.

efriedma added inline comments.Fri, Apr 13, 2:00 PM
lib/Transforms/Utils/BuildLibCalls.cpp
1062 ↗(On Diff #142247)

This isn't right; should be TLI->has(LibFunc_calloc).

xbolva00 updated this revision to Diff 142488.Fri, Apr 13, 4:31 PM
xbolva00 added inline comments.
lib/Transforms/Utils/BuildLibCalls.cpp
1062 ↗(On Diff #142247)

Fixed.

Anyway, emitCalloc function I just copied from SimplifyLibCalls since it has no sense to have it there. So some time ago when it was merged and there was without proper review?

xbolva00 marked an inline comment as done.Mon, Apr 16, 6:35 AM

Any review or merge? @spatel ?

xbolva00 accepted this revision.Mon, Apr 16, 9:27 AM
This revision is now accepted and ready to land.Mon, Apr 16, 9:27 AM

LGTM

I have no write access, so somebody must merge it. @spatel ?

The patch doesn't compile as-is. Please post a corrected patch.

/llvm/lib/Transforms/Utils/BuildLibCalls.cpp:1061:11: error: member reference type 'const llvm::TargetLibraryInfo' is not a pointer; did you mean to use
      '.'?
  if (!TLI->has(LibFunc_calloc))
       ~~~^~
          .
/llvm/lib/Transforms/Utils/BuildLibCalls.cpp:1069:53: error: indirection requires pointer operand ('const llvm::TargetLibraryInfo' invalid)
  inferLibFuncAttributes(*M->getFunction("calloc"), *TLI);
xbolva00 updated this revision to Diff 142881.Tue, Apr 17, 10:12 PM
This revision was automatically updated to reflect the committed changes.