Page MenuHomePhabricator

[GlobalISel] Avoid making G_PTR_ADD with nullptr
ClosedPublic

Authored by mbrkusanin on Sep 4 2020, 6:04 AM.

Details

Summary

When the first operand is a null pointer we can avoid making a G_PTR_ADD and
make a G_INTTOPTR with the offset operand.
This helps us avoid making add with 0 later on for targets such as AMDGPU.

Diff Detail

Event Timeline

mbrkusanin created this revision.Sep 4 2020, 6:04 AM
mbrkusanin requested review of this revision.Sep 4 2020, 6:04 AM
arsenm added a comment.Sep 4 2020, 6:46 AM

This is probably wrong for non integral pointers. Why not do this as a combine later? We need to start recognizing G_PTRADD (constant) + constant anyway to handle lowered LDS addresses

foad added inline comments.Sep 4 2020, 7:12 AM
llvm/test/CodeGen/AArch64/GlobalISel/translate-gep.ll
11 ↗(On Diff #289930)

Can you avoid generating this unused constant in the first place?

  • Reimplemented as a AMDGPU GICombineRule.
arsenm added inline comments.Sep 16 2020, 6:13 AM
llvm/lib/Target/AMDGPU/AMDGPUCombine.td
40 ↗(On Diff #292192)

ptr_add

40–45 ↗(On Diff #292192)

This should go in the generic combiner

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp
45 ↗(On Diff #292192)

Should not construct a local MachineIRBuilder

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-add-nullptr.mir
53–54

Can simplify these out of the test

57

Should add vector test

arsenm added inline comments.Tue, Sep 29, 7:31 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
2737–2740

Combine the scalar and vector check above?

2743

I have a patch to generalize these checks I haven't posted yet, but I guess having the two paths is fine for now

  • Refactored matchPtrAddZero().
mbrkusanin marked an inline comment as done.Tue, Sep 29, 7:53 AM
  • Rebase
  • Ping
foad added inline comments.Tue, Oct 13, 2:27 AM
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
2774

This is just Ty.getScalarType().

2783

You should be able to assert that it is a vector here.

mbrkusanin marked 2 inline comments as done.
  • Addressed comments.
foad accepted this revision.Tue, Oct 13, 2:57 AM

LGTM.

This revision is now accepted and ready to land.Tue, Oct 13, 2:57 AM
This revision was automatically updated to reflect the committed changes.