This is an archive of the discontinued LLVM Phabricator instance.

X86WinAllocaExpander: Drop code looking through register copies (PR41786)
ClosedPublic

Authored by hans on May 8 2019, 3:06 AM.

Diff Detail

Event Timeline

hans created this revision.May 8 2019, 3:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2019, 3:06 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
RKSimon added inline comments.May 8 2019, 3:30 AM
llvm/lib/Target/X86/X86WinAllocaExpander.cpp
269

Can this be null? We were testing for it before and still do above.

hans updated this revision to Diff 198624.May 8 2019, 4:38 AM
hans marked 2 inline comments as done.
hans added inline comments.
llvm/lib/Target/X86/X86WinAllocaExpander.cpp
269

Yes, we should keep that. Good catch.

One last comment from me - @rnk please can you take a look? I know very little about this code....

llvm/lib/Target/X86/X86WinAllocaExpander.cpp
272

(style) drop the braces

rnk accepted this revision.May 8 2019, 11:11 AM

lgtm, too

llvm/lib/Target/X86/X86WinAllocaExpander.cpp
91–94

As a follow-up, I think it would be reasonable for WIN_ALLOCA_* to accept either reg or imm operands so we won't need this code at all. Right now we emit this MI:

%12:gr32 = MOV32ri 12
WIN_ALLOCA_32 %12:gr32, implicit-def dead $eax, implicit-def dead $esp, implicit-def dead $eflags, implicit $esp

But it's not really necessary, since in the end we're going to make SUB64ri instructions with the immediate.

I think the instruction wasn't designed this way because most truly dynamic allocas do not use a fixed size. It's only inalloca that introduces them.

This revision is now accepted and ready to land.May 8 2019, 11:11 AM
This revision was automatically updated to reflect the committed changes.
hans marked an inline comment as done.