This is an archive of the discontinued LLVM Phabricator instance.

[X86] Simplify isReplaceable (NFC)
Needs ReviewPublic

Authored by kazu on Sep 11 2022, 11:43 AM.

Details

Summary

isReplaceable checks to see if every use of the second LEA (called
"Last" in the function) is as the base address of a respective memory
access instruction.

The "for" loop being deleted in this patch potentially checks every
operand of the use instruction.

The patch simplifies the code by checking to see if every use is as
the base address of a respective memory access instruction, which we
can do with a quick pointer comparison.

Diff Detail

Event Timeline

kazu created this revision.Sep 11 2022, 11:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2022, 11:43 AM
kazu requested review of this revision.Sep 11 2022, 11:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2022, 11:43 AM
kazu added a comment.Sep 13 2022, 9:25 AM

Please take a look. Thanks!

pengfei added inline comments.Sep 13 2022, 7:11 PM
llvm/lib/Target/X86/X86OptimizeLEAs.cpp
464

This cannot prevent def register been used by other operands at the same time.

kazu added inline comments.Sep 13 2022, 9:36 PM
llvm/lib/Target/X86/X86OptimizeLEAs.cpp
464

This cannot prevent def register been used by other operands at the same time.

Sure, a single iteration of the "for" loop may not prevent the def register being used by other operands, but if we check to see if all uses are used as the address base register, then no use is left for any other purpose. Or it it possible that some uses are not counted in the use list?

pengfei added inline comments.Sep 13 2022, 10:44 PM
llvm/lib/Target/X86/X86OptimizeLEAs.cpp
464

I see your point. But I don't know. I have some concerns, e.g., if their pointer are always unique? It is fragile given not guaranteed by API/doc. And if pointer comparison is stable enough, why is isIdenticalTo implemented in a complex way?