Page MenuHomePhabricator

GlobalISel: Constant fold G_PTR_ADD
AcceptedPublic

Authored by arsenm on Mar 21 2021, 7:03 AM.

Details

Summary

Some globals lower to literal addresses on AMDGPU.

This may be wrong for non-integral address spaces. I'm wondering if we
should just allow regular G_ADD to use pointer types, and reserve
G_PTR_ADD for non-integral address spaces.

Diff Detail

Event Timeline

arsenm created this revision.Mar 21 2021, 7:03 AM
arsenm requested review of this revision.Mar 21 2021, 7:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2021, 7:03 AM
Herald added a subscriber: wdng. · View Herald Transcript
paquette accepted this revision.Mar 23 2021, 9:53 AM

LGTM

I'm wondering if we should just allow regular G_ADD to use pointer types, and reserve G_PTR_ADD for non-integral address spaces.

Maybe? We've had a few cases with AArch64 where we miss optimizations because we forget to handle both G_ADD and G_PTR_ADD.

I guess that knowing immediately that we have a pointer add is nice for matching etc, but I guess we also know that from the LLT.

This revision is now accepted and ready to land.Mar 23 2021, 9:53 AM

Relaxing G_ADD semantics seems fine to me. Maybe we should still canonicalize on G_PTR_ADD during translation, and allow targets to legalize them into plain G_ADDs.

Relaxing G_ADD semantics seems fine to me. Maybe we should still canonicalize on G_PTR_ADD during translation, and allow targets to legalize them into plain G_ADDs.

IRTranslator should definitely translate to G_PTR_ADD and G_PTR_ADD should not be considered equivalent to G_ADD. However for targets where pointers are just integers I see no harm in permitting G_ADD of pointers and targets custom-legalizing/lowering to that. It just shouldn't be something that happens without the targets actively choosing to do it.

llvm/lib/CodeGen/GlobalISel/Utils.cpp
446

I don't think we should add constant folding for G_PTR_ADD since it might not be an integer and might even be addrspace dependent behaviour. For integral pointers we should lower to G_ADD first, and then use G_ADD's constant folding.