Page MenuHomePhabricator

[PowerPC][Future] Add initial support for PC Relative addressing for global values

Authored by stefanp on Feb 27 2020, 11:27 AM.



This patch adds PC Relative support for global values that are known at link time.
If a global value requires access through the global offset table (GOT) it is not covered in this patch.

Diff Detail

Event Timeline

stefanp created this revision.Feb 27 2020, 11:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2020, 11:27 AM
Herald added subscribers: shchenz, jsji, jfb and 2 others. · View Herald Transcript
stefanp updated this revision to Diff 249420.Mar 10 2020, 9:29 AM

Removed inaccurate debug statement.

lei added inline comments.Mar 10 2020, 1:53 PM

Why is this code move needed?

stefanp updated this revision to Diff 249924.Mar 12 2020, 7:15 AM

It is correct that the code did not need to be moved. I've moved it back.

amyk added a subscriber: amyk.Mar 14 2020, 3:11 PM
amyk added inline comments.

Does it make sense to also add that it is a PC Relative address if it's marked with the PPCII::MO_PCREL_FLAG or if the node opcode is PPCISD::MAT_PCREL_ADDR?

nemanjai accepted this revision.Mar 17 2020, 6:38 AM

My comments are quite minor so feel free to address them on the commit. LGTM.


The logic might be easier to follow if we do something like:

Base = N;
if (N.getOpcode() == PPCISD::MAT_PCREL_ADDR)
  return true;
if (ConstantPoolSDNode *CPN = dyn_cast<ConstantPoolSDNode>(N) &&
                              (CPN->getTargetFlags() & PPCII::MO_PCREL_FLAG))
  return true;
if (GlobalAddressSDNode *GAN = dyn_cast<GlobalAddressSDNode>(N) &&
                               (GAN->getTargetFlags() & PPCII::MO_PCREL_FLAG))
  return true;
return false;

Please remove this from the AIX block. There is no current plan to support PC-relative addressing on AIX.

This revision is now accepted and ready to land.Mar 17 2020, 6:38 AM
anil9 added a subscriber: anil9.Mar 25 2020, 9:05 PM
anil9 added inline comments.

There is a slight change in logic with the above code. In the current implementation Base will not be set to N if the result is false, but with the code in the comment it will always be set to N.

anil9 added inline comments.Apr 4 2020, 7:29 PM

The naming convention is a bit inconsistent :

GSDN / ConstPoolNode , this inconsistency is carried over this patch too.

JumpTableSDNode *JT = dyn_cast<JumpTableSDNode>(N.getNode());

This revision was automatically updated to reflect the committed changes.