[PowerPC] option -msoft-float should not block the PC-relative address instruction
#92543
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-powerpc Author: zhijian lin (diggerlin) ChangesThe PR will fix the issue Clang powerpc64le pcrel code model is not used if -msoft-float or similar flags are used Full diff: https://github.com/llvm/llvm-project/pull/92543.diff 4 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index b962ed28d7200..31441fd4f0d6b 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -296,8 +296,7 @@ def FeatureVectorsUseTwoUnits : SubtargetFeature<"vectors-use-two-units",
def FeaturePrefixInstrs : SubtargetFeature<"prefix-instrs", "HasPrefixInstrs",
"true",
"Enable prefixed instructions",
- [FeatureISA3_0, FeatureP8Vector,
- FeatureP9Altivec]>;
+ [FeatureISA3_1]>;
def FeaturePCRelativeMemops :
SubtargetFeature<"pcrelative-memops", "HasPCRelativeMemops", "true",
"Enable PC relative Memory Ops",
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index d27932f2915fb..855927ec8b3ed 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -18258,9 +18258,11 @@ unsigned PPCTargetLowering::computeMOFlags(const SDNode *Parent, SDValue N,
FlagSet |= PPC::MOF_SubtargetBeforeP9;
else {
FlagSet |= PPC::MOF_SubtargetP9;
- if (Subtarget.hasPrefixInstrs())
- FlagSet |= PPC::MOF_SubtargetP10;
}
+
+ if (Subtarget.hasPrefixInstrs())
+ FlagSet |= PPC::MOF_SubtargetP10;
+
if (Subtarget.hasSPE())
FlagSet |= PPC::MOF_SubtargetSPE;
diff --git a/llvm/lib/Target/PowerPC/PPCInstrP10.td b/llvm/lib/Target/PowerPC/PPCInstrP10.td
index 5f2937d47a519..b76f1419df778 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrP10.td
+++ b/llvm/lib/Target/PowerPC/PPCInstrP10.td
@@ -654,13 +654,10 @@ let Predicates = [PrefixInstrs] in {
(ins s34imm:$SI),
"pli $RT, $SI", IIC_IntSimple, []>;
}
+}
+let Predicates = [PrefixInstrs, HasFPU] in {
let mayLoad = 1, mayStore = 0 in {
- defm PLXV :
- 8LS_DForm_R_SI34_XT6_RA5_MEM_p<25, (outs vsrc:$XST), (ins (memri34 $D, $RA):$addr),
- (ins (memri34_pcrel $D, $RA):$addr),
- (ins s34imm_pcrel:$D),
- "plxv $XST, $addr", "plxv $XST, $D", IIC_LdStLFD>;
defm PLFS :
MLS_DForm_R_SI34_RTA5_MEM_p<48, (outs f4rc:$RST), (ins (memri34 $D, $RA):$addr),
(ins (memri34_pcrel $D, $RA):$addr),
@@ -671,6 +668,28 @@ let Predicates = [PrefixInstrs] in {
(ins (memri34_pcrel $D, $RA):$addr),
(ins s34imm_pcrel:$D), "plfd $RST, $addr",
"plfd $RST, $D", IIC_LdStLFD>;
+ }
+ let mayStore = 1, mayLoad = 0 in {
+ defm PSTFS :
+ MLS_DForm_R_SI34_RTA5_MEM_p<52, (outs), (ins f4rc:$RST, (memri34 $D, $RA):$addr),
+ (ins f4rc:$RST, (memri34_pcrel $D, $RA):$addr),
+ (ins f4rc:$RST, s34imm_pcrel:$D),
+ "pstfs $RST, $addr", "pstfs $RST, $D", IIC_LdStLFD>;
+ defm PSTFD :
+ MLS_DForm_R_SI34_RTA5_MEM_p<54, (outs), (ins f8rc:$RST, (memri34 $D, $RA):$addr),
+ (ins f8rc:$RST, (memri34_pcrel $D, $RA):$addr),
+ (ins f8rc:$RST, s34imm_pcrel:$D),
+ "pstfd $RST, $addr", "pstfd $RST, $D", IIC_LdStLFD>;
+ }
+}
+
+let Predicates = [PrefixInstrs, HasP10Vector] in {
+ let mayLoad = 1, mayStore = 0 in {
+ defm PLXV :
+ 8LS_DForm_R_SI34_XT6_RA5_MEM_p<25, (outs vsrc:$XST), (ins (memri34 $D, $RA):$addr),
+ (ins (memri34_pcrel $D, $RA):$addr),
+ (ins s34imm_pcrel:$D),
+ "plxv $XST, $addr", "plxv $XST, $D", IIC_LdStLFD>;
defm PLXSSP :
8LS_DForm_R_SI34_RTA5_MEM_p<43, (outs vfrc:$RST), (ins (memri34 $D, $RA):$addr),
(ins (memri34_pcrel $D, $RA):$addr),
@@ -683,6 +702,28 @@ let Predicates = [PrefixInstrs] in {
(ins s34imm_pcrel:$D),
"plxsd $RST, $addr", "plxsd $RST, $D",
IIC_LdStLFD>;
+ }
+ let mayStore = 1, mayLoad = 0 in {
+ defm PSTXV :
+ 8LS_DForm_R_SI34_XT6_RA5_MEM_p<27, (outs), (ins vsrc:$XST, (memri34 $D, $RA):$addr),
+ (ins vsrc:$XST, (memri34_pcrel $D, $RA):$addr),
+ (ins vsrc:$XST, s34imm_pcrel:$D),
+ "pstxv $XST, $addr", "pstxv $XST, $D", IIC_LdStLFD>;
+ defm PSTXSSP :
+ 8LS_DForm_R_SI34_RTA5_MEM_p<47, (outs), (ins vfrc:$RST, (memri34 $D, $RA):$addr),
+ (ins vfrc:$RST, (memri34_pcrel $D, $RA):$addr),
+ (ins vfrc:$RST, s34imm_pcrel:$D),
+ "pstxssp $RST, $addr", "pstxssp $RST, $D", IIC_LdStLFD>;
+ defm PSTXSD :
+ 8LS_DForm_R_SI34_RTA5_MEM_p<46, (outs), (ins vfrc:$RST, (memri34 $D, $RA):$addr),
+ (ins vfrc:$RST, (memri34_pcrel $D, $RA):$addr),
+ (ins vfrc:$RST, s34imm_pcrel:$D),
+ "pstxsd $RST, $addr", "pstxsd $RST, $D", IIC_LdStLFD>;
+ }
+}
+
+let Predicates = [PrefixInstrs] in {
+ let mayLoad = 1, mayStore = 0 in {
let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
defm PLBZ8 :
MLS_DForm_R_SI34_RTA5_MEM_p<34, (outs g8rc:$RST), (ins (memri34 $D, $RA):$addr),
@@ -745,31 +786,6 @@ let Predicates = [PrefixInstrs] in {
}
let mayStore = 1, mayLoad = 0 in {
- defm PSTXV :
- 8LS_DForm_R_SI34_XT6_RA5_MEM_p<27, (outs), (ins vsrc:$XST, (memri34 $D, $RA):$addr),
- (ins vsrc:$XST, (memri34_pcrel $D, $RA):$addr),
- (ins vsrc:$XST, s34imm_pcrel:$D),
- "pstxv $XST, $addr", "pstxv $XST, $D", IIC_LdStLFD>;
- defm PSTFS :
- MLS_DForm_R_SI34_RTA5_MEM_p<52, (outs), (ins f4rc:$RST, (memri34 $D, $RA):$addr),
- (ins f4rc:$RST, (memri34_pcrel $D, $RA):$addr),
- (ins f4rc:$RST, s34imm_pcrel:$D),
- "pstfs $RST, $addr", "pstfs $RST, $D", IIC_LdStLFD>;
- defm PSTFD :
- MLS_DForm_R_SI34_RTA5_MEM_p<54, (outs), (ins f8rc:$RST, (memri34 $D, $RA):$addr),
- (ins f8rc:$RST, (memri34_pcrel $D, $RA):$addr),
- (ins f8rc:$RST, s34imm_pcrel:$D),
- "pstfd $RST, $addr", "pstfd $RST, $D", IIC_LdStLFD>;
- defm PSTXSSP :
- 8LS_DForm_R_SI34_RTA5_MEM_p<47, (outs), (ins vfrc:$RST, (memri34 $D, $RA):$addr),
- (ins vfrc:$RST, (memri34_pcrel $D, $RA):$addr),
- (ins vfrc:$RST, s34imm_pcrel:$D),
- "pstxssp $RST, $addr", "pstxssp $RST, $D", IIC_LdStLFD>;
- defm PSTXSD :
- 8LS_DForm_R_SI34_RTA5_MEM_p<46, (outs), (ins vfrc:$RST, (memri34 $D, $RA):$addr),
- (ins vfrc:$RST, (memri34_pcrel $D, $RA):$addr),
- (ins vfrc:$RST, s34imm_pcrel:$D),
- "pstxsd $RST, $addr", "pstxsd $RST, $D", IIC_LdStLFD>;
let Interpretation64Bit = 1, isCodeGenOnly = 1 in {
defm PSTB8 :
MLS_DForm_R_SI34_RTA5_MEM_p<38, (outs), (ins g8rc:$RST, (memri34 $D, $RA):$addr),
@@ -1136,7 +1152,7 @@ let mayLoad = 0, mayStore = 1, Predicates = [PairedVectorMemops] in {
[]>;
}
-let mayLoad = 1, mayStore = 0, Predicates = [PairedVectorMemops, PrefixInstrs] in {
+let mayLoad = 1, mayStore = 0, Predicates = [PairedVectorMemops, PrefixInstrs, HasP10Vector] in {
defm PLXVP :
8LS_DForm_R_XTp5_SI34_MEM_p<58, (outs vsrprc:$XTp), (ins (memri34 $D, $RA):$addr),
(ins (memri34_pcrel $D, $RA):$addr),
@@ -1145,7 +1161,7 @@ let mayLoad = 1, mayStore = 0, Predicates = [PairedVectorMemops, PrefixInstrs] i
IIC_LdStLFD>;
}
-let mayLoad = 0, mayStore = 1, Predicates = [PairedVectorMemops, PrefixInstrs] in {
+let mayLoad = 0, mayStore = 1, Predicates = [PairedVectorMemops, PrefixInstrs, HasP10Vector] in {
defm PSTXVP :
8LS_DForm_R_XTp5_SI34_MEM_p<62, (outs), (ins vsrprc:$XTp, (memri34 $D, $RA):$addr),
(ins vsrprc:$XTp, (memri34_pcrel $D, $RA):$addr),
@@ -1157,7 +1173,7 @@ let Predicates = [PairedVectorMemops] in {
// Intrinsics for Paired Vector Loads.
def : Pat<(v256i1 (int_ppc_vsx_lxvp DQForm:$src)), (LXVP memrix16:$src)>;
def : Pat<(v256i1 (int_ppc_vsx_lxvp XForm:$src)), (LXVPX XForm:$src)>;
- let Predicates = [PairedVectorMemops, PrefixInstrs] in {
+ let Predicates = [PairedVectorMemops, PrefixInstrs, HasP10Vector] in {
def : Pat<(v256i1 (int_ppc_vsx_lxvp PDForm:$src)), (PLXVP memri34:$src)>;
}
// Intrinsics for Paired Vector Stores.
@@ -1165,7 +1181,7 @@ let Predicates = [PairedVectorMemops] in {
(STXVP $XSp, memrix16:$dst)>;
def : Pat<(int_ppc_vsx_stxvp v256i1:$XSp, XForm:$dst),
(STXVPX $XSp, XForm:$dst)>;
- let Predicates = [PairedVectorMemops, PrefixInstrs] in {
+ let Predicates = [PairedVectorMemops, PrefixInstrs, HasP10Vector] in {
def : Pat<(int_ppc_vsx_stxvp v256i1:$XSp, PDForm:$dst),
(PSTXVP $XSp, memri34:$dst)>;
}
diff --git a/llvm/test/CodeGen/PowerPC/issue-62372-bug.ll b/llvm/test/CodeGen/PowerPC/issue-62372-bug.ll
new file mode 100644
index 0000000000000..12c4afc56cdb8
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/issue-62372-bug.ll
@@ -0,0 +1,15 @@
+; RUN: llc -o - %s| FileCheck %s
+
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@bar = dso_local global i32 0, align 4
+
+define dso_local ptr @foo() #0 {
+entry:
+ ret ptr @bar
+}
+
+attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pwr10" "target-features"="+altivec,+bpermd,+crbits,+crypto,+direct-move,+extdiv,+isa-v206-instructions,+isa-v207-instructions,+isa-v30-instructions,+isa-v31-instructions,+mma,+paired-vector-memops,+pcrelative-memops,+power10-vector,+power8-vector,+power9-vector,+prefix-instrs,+quadword-atomics,+vsx,-aix-small-local-dynamic-tls,-aix-small-local-exec-tls,-hard-float,-htm,-privileged,-rop-protect,-spe" "use-soft-float"="true" }
+
+
+; CHECK: paddi 3, 0, bar@PCREL, 1
|
diggerlin
requested review from
chenzheng1030,
stefanp-ibm,
mandlebug,
kamaub,
lei137,
amy-kwan and
syzaara
There was a problem hiding this comment.
Thanks for improving this.
- In the description,
Fixes #62372should be a right form to connect this pr and the issue. - We may also need to check all the places where
hasPrefixInstrs()is called. NowhasPrefixInstrs()does not necessary mean vector instructions are also enabled, so we need to add check forhasP10Vector()too. For example for the usages inllvm/lib/Target/PowerPC/PPCRegisterInfo.cppllvm/lib/Target/PowerPC/PPCISelLowering.cpp
| @@ -18258,9 +18258,11 @@ unsigned PPCTargetLowering::computeMOFlags(const SDNode *Parent, SDValue N, | |||
| FlagSet |= PPC::MOF_SubtargetBeforeP9; | |||
| else { | |||
There was a problem hiding this comment.
nit: the {} can be removed now?
|
|
||
| define dso_local ptr @foo() #0 { | ||
| entry: | ||
| ret ptr @bar |
There was a problem hiding this comment.
nit, the format here
| ret ptr @bar | ||
| } | ||
|
|
||
| attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="pwr10" "target-features"="+altivec,+bpermd,+crbits,+crypto,+direct-move,+extdiv,+isa-v206-instructions,+isa-v207-instructions,+isa-v30-instructions,+isa-v31-instructions,+mma,+paired-vector-memops,+pcrelative-memops,+power10-vector,+power8-vector,+power9-vector,+prefix-instrs,+quadword-atomics,+vsx,-aix-small-local-dynamic-tls,-aix-small-local-exec-tls,-hard-float,-htm,-privileged,-rop-protect,-spe" "use-soft-float"="true" } |
There was a problem hiding this comment.
Is it possible to tune the attributes a little bit? Just use the ones that matter? I don't believe the "frame-pointer"="all" "no-trapping-math"="true" will impact the result.
| @@ -0,0 +1,15 @@ | |||
| ; RUN: llc -o - %s| FileCheck %s | |||
There was a problem hiding this comment.
nit: we usually use name pr62372.ll for a fix targeting for a github issue.
amy-kwan
reviewed
May 21, 2024
| @@ -0,0 +1,15 @@ | |||
| ; RUN: llc -o - %s| FileCheck %s | |||
There was a problem hiding this comment.
nit: It would be good to add ppc-asm-full-reg-names to the RUN line.
llvm/test/CodeGen/PowerPC/pr92543.ll
Outdated
| ret ptr @bar | ||
| } | ||
|
|
||
| attributes #0 = { noinline nounwind optnone uwtable "target-cpu"="pwr10" "target-features"="+altivec,+isa-v30-instructions,+isa-v31-instructions,+pcrelative-memops,+power10-vector,+power9-vector,+prefix-instrs,+vsx,-hard-float" "use-soft-float"="true" } |
There was a problem hiding this comment.
Will the attribute:
attributes #0 = {"use-soft-float"="true"} and adding -mcpu=pwr10 in the run line get the expected output?
llvm/test/CodeGen/PowerPC/pr92543.ll
Outdated
| @@ -0,0 +1,15 @@ | |||
| ; RUN: llc -ppc-asm-full-reg-names -o - %s| FileCheck %s | |||
|
|
|||
| target triple = "powerpc64le-unknown-linux-gnu" | |||
There was a problem hiding this comment.
nit: Maybe better to put the triple in the run line. So the case is able to extend to other targets.
| } | ||
| } | ||
|
|
||
| let Predicates = [PrefixInstrs] in { |
There was a problem hiding this comment.
Maybe we can add a follow up patch to group the instruction/pattern defs by the Predicates. At least [PrefixInstrs] and [PrefixInstrs, HasP10Vector] are used in more than 1 place. We can do this later.
There was a problem hiding this comment.
I will create a NFC patch for it later.
llvm/test/CodeGen/PowerPC/pr92543.ll
Outdated
| @@ -0,0 +1,15 @@ | |||
| ; RUN: llc -ppc-asm-full-reg-names -o - %s| FileCheck %s | |||
There was a problem hiding this comment.
the file name should contain the bug issue number 62372, not the patch number.(a little confusing : ))
amy-kwan
reviewed
May 23, 2024
llvm/test/CodeGen/PowerPC/pr62372.ll
Outdated
| @@ -0,0 +1,12 @@ | |||
| ; RUN: llc -ppc-asm-full-reg-names -mcpu=pwr10 -mtriple powerpc64le-unknown-linux-gnu -o - %s| FileCheck %s | |||
There was a problem hiding this comment.
nit: Line too long. Can we split up the line?
chenzheng1030
approved these changes
May 24, 2024
There was a problem hiding this comment.
LGTM as long as @amy-kwan 's comment is addressed.
Please wait for some days for other reviewers.
Thanks very much for the improvements.
diggerlin
added a commit
that referenced
this pull request
Jun 3, 2024
reorganize the PPCInstrP10.td based on comment #92543 (comment) The instructions or patterns defined by same predicates are currently placed at several different locations , They will be reorganized into same group based on these predicates in the patch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Prefix instruction is introduced on PowerPC ISA3_1.
In the PR,
FeaturePrefixInstrsdo not imply theFeatureP8Vector,FeatureP9Vector.FeaturePrefixInstrsimplies only the FeatureISA3_1.paddiandpli, they havePredicates = [PrefixInstrs]plfsandplfd, they havePredicates = [PrefixInstrs, HasFPU], "plxsspandplxsd, they havePredicates = [PrefixInstrs, HasP10Vector]Fixes #62372