-
Notifications
You must be signed in to change notification settings - Fork 15.4k
PPC: Split 64bit target feature into 64bit and 64bit-support #157206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-backend-powerpc Author: Matt Arsenault (arsenm) ChangesThis was being used for 2 different purposes. The TargetMachine constructor prepends +64bit based on isPPC64 The comment in tablegen suggests it's relevant to track which processors Full diff: https://github.com/llvm/llvm-project/pull/157206.diff 4 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index db6427cfe8482..386d0f65d1ed1 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -58,8 +58,13 @@ def DirectivePwrFuture
// Specifies that the selected CPU supports 64-bit instructions, regardless of
// whether we are in 32-bit or 64-bit mode.
-def Feature64Bit : SubtargetFeature<"64bit","Has64BitSupport", "true",
- "Enable 64-bit instructions">;
+def Feature64BitSupport : SubtargetFeature<"64bit-support", "Has64BitSupport", "true",
+ "Supports 64-bit instructions">;
+// 64-bit is enabled.
+def Feature64Bit : SubtargetFeature<"64bit", "IsPPC64", "true",
+ "Enable 64-bit mode",
+ [Feature64BitSupport]>;
+
def AIXOS: SubtargetFeature<"aix", "IsAIX", "true", "AIX OS">;
def FeatureModernAIXAs
: SubtargetFeature<"modern-aix-as", "HasModernAIXAs", "true",
@@ -85,7 +90,7 @@ def FeatureAltivec : SubtargetFeature<"altivec","HasAltivec", "true",
def FeatureSPE : SubtargetFeature<"spe","HasSPE", "true",
"Enable SPE instructions",
[FeatureHardFloat]>;
-def FeatureEFPU2 : SubtargetFeature<"efpu2", "HasEFPU2", "true",
+def FeatureEFPU2 : SubtargetFeature<"efpu2", "HasEFPU2", "true",
"Enable Embedded Floating-Point APU 2 instructions",
[FeatureSPE]>;
def FeatureMFOCRF : SubtargetFeature<"mfocrf","HasMFOCRF", "true",
@@ -429,7 +434,7 @@ def ProcessorFeatures {
FeaturePOPCNTD,
FeatureCMPB,
FeatureLDBRX,
- Feature64Bit,
+ Feature64BitSupport,
/* Feature64BitRegs, */
FeatureBPERMD,
FeatureExtDiv,
@@ -667,13 +672,13 @@ def : ProcessorModel<"970", G5Model,
[Directive970, FeatureAltivec,
FeatureMFOCRF, FeatureFSqrt,
FeatureFRES, FeatureFRSQRTE, FeatureSTFIWX,
- Feature64Bit /*, Feature64BitRegs */,
+ Feature64BitSupport /*, Feature64BitRegs */,
FeatureMFTB]>;
def : ProcessorModel<"g5", G5Model,
[Directive970, FeatureAltivec,
FeatureMFOCRF, FeatureFSqrt, FeatureSTFIWX,
FeatureFRES, FeatureFRSQRTE,
- Feature64Bit /*, Feature64BitRegs */,
+ Feature64BitSupport /*, Feature64BitRegs */,
FeatureMFTB, DeprecatedDST]>;
def : ProcessorModel<"e500", PPCE500Model,
[DirectiveE500,
@@ -694,41 +699,41 @@ def : ProcessorModel<"a2", PPCA2Model,
FeatureSTFIWX, FeatureLFIWAX,
FeatureFPRND, FeatureFPCVT, FeatureISEL,
FeatureSlowPOPCNTD, FeatureCMPB, FeatureLDBRX,
- Feature64Bit /*, Feature64BitRegs */, FeatureMFTB,
+ Feature64BitSupport /*, Feature64BitRegs */, FeatureMFTB,
FeatureISA2_06]>;
def : ProcessorModel<"pwr3", G5Model,
[DirectivePwr3, FeatureAltivec,
FeatureFRES, FeatureFRSQRTE, FeatureMFOCRF,
- FeatureSTFIWX, Feature64Bit]>;
+ FeatureSTFIWX, Feature64BitSupport]>;
def : ProcessorModel<"pwr4", G5Model,
[DirectivePwr4, FeatureAltivec, FeatureMFOCRF,
FeatureFSqrt, FeatureFRES, FeatureFRSQRTE,
- FeatureSTFIWX, Feature64Bit, FeatureMFTB]>;
+ FeatureSTFIWX, Feature64BitSupport, FeatureMFTB]>;
def : ProcessorModel<"pwr5", G5Model,
[DirectivePwr5, FeatureAltivec, FeatureMFOCRF,
FeatureFSqrt, FeatureFRE, FeatureFRES,
FeatureFRSQRTE, FeatureFRSQRTES,
- FeatureSTFIWX, Feature64Bit,
+ FeatureSTFIWX, Feature64BitSupport,
FeatureMFTB, DeprecatedDST]>;
def : ProcessorModel<"pwr5x", G5Model,
[DirectivePwr5x, FeatureAltivec, FeatureMFOCRF,
FeatureFSqrt, FeatureFRE, FeatureFRES,
FeatureFRSQRTE, FeatureFRSQRTES,
- FeatureSTFIWX, FeatureFPRND, Feature64Bit,
+ FeatureSTFIWX, FeatureFPRND, Feature64BitSupport,
FeatureMFTB, DeprecatedDST]>;
def : ProcessorModel<"pwr6", G5Model,
[DirectivePwr6, FeatureAltivec,
FeatureMFOCRF, FeatureFCPSGN, FeatureFSqrt, FeatureFRE,
FeatureFRES, FeatureFRSQRTE, FeatureFRSQRTES,
FeatureRecipPrec, FeatureSTFIWX, FeatureLFIWAX, FeatureCMPB,
- FeatureFPRND, Feature64Bit /*, Feature64BitRegs */,
+ FeatureFPRND, Feature64BitSupport /*, Feature64BitRegs */,
FeatureMFTB, DeprecatedDST]>;
def : ProcessorModel<"pwr6x", G5Model,
[DirectivePwr5x, FeatureAltivec, FeatureMFOCRF,
FeatureFCPSGN, FeatureFSqrt, FeatureFRE, FeatureFRES,
FeatureFRSQRTE, FeatureFRSQRTES, FeatureRecipPrec,
FeatureSTFIWX, FeatureLFIWAX, FeatureCMPB,
- FeatureFPRND, Feature64Bit,
+ FeatureFPRND, Feature64BitSupport,
FeatureMFTB, DeprecatedDST]>;
def : ProcessorModel<"pwr7", P7Model, ProcessorFeatures.P7Features>;
def : ProcessorModel<"pwr8", P8Model, ProcessorFeatures.P8Features>;
@@ -746,7 +751,7 @@ def : ProcessorModel<"ppc64", G5Model,
[Directive64, FeatureAltivec,
FeatureMFOCRF, FeatureFSqrt, FeatureFRES,
FeatureFRSQRTE, FeatureSTFIWX,
- Feature64Bit /*, Feature64BitRegs */,
+ Feature64BitSupport /*, Feature64BitRegs */,
FeatureMFTB]>;
def : ProcessorModel<"ppc64le", P8Model, ProcessorFeatures.P8Features>;
diff --git a/llvm/lib/Target/PowerPC/PPCSubtarget.cpp b/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
index 996b6efb320df..c63c6142a6799 100644
--- a/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ b/llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -55,10 +55,8 @@ PPCSubtarget &PPCSubtarget::initializeSubtargetDependencies(StringRef CPU,
PPCSubtarget::PPCSubtarget(const Triple &TT, const std::string &CPU,
const std::string &TuneCPU, const std::string &FS,
const PPCTargetMachine &TM)
- : PPCGenSubtargetInfo(TT, CPU, TuneCPU, FS), TargetTriple(TT),
- IsPPC64(TargetTriple.getArch() == Triple::ppc64 ||
- TargetTriple.getArch() == Triple::ppc64le),
- TM(TM), FrameLowering(initializeSubtargetDependencies(CPU, TuneCPU, FS)),
+ : PPCGenSubtargetInfo(TT, CPU, TuneCPU, FS), TargetTriple(TT), TM(TM),
+ FrameLowering(initializeSubtargetDependencies(CPU, TuneCPU, FS)),
InstrInfo(*this), TLInfo(TM, *this) {
TSInfo = std::make_unique<PPCSelectionDAGInfo>();
@@ -248,7 +246,6 @@ CodeModel::Model PPCSubtarget::getCodeModel(const TargetMachine &TM,
}
bool PPCSubtarget::isELFv2ABI() const { return TM.isELFv2ABI(); }
-bool PPCSubtarget::isPPC64() const { return TM.isPPC64(); }
bool PPCSubtarget::isUsingPCRelativeCalls() const {
return isPPC64() && hasPCRelativeMemops() && isELFv2ABI() &&
diff --git a/llvm/lib/Target/PowerPC/PPCSubtarget.h b/llvm/lib/Target/PowerPC/PPCSubtarget.h
index 3c59a475c7eb6..24e464b1e40f9 100644
--- a/llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ b/llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -96,7 +96,6 @@ class PPCSubtarget : public PPCGenSubtargetInfo {
/// Which cpu directive was used.
unsigned CPUDirective;
- bool IsPPC64;
bool IsLittleEndian;
POPCNTDKind HasPOPCNTD;
@@ -171,10 +170,6 @@ class PPCSubtarget : public PPCGenSubtargetInfo {
void initSubtargetFeatures(StringRef CPU, StringRef TuneCPU, StringRef FS);
public:
- /// isPPC64 - Return true if we are generating code for 64-bit pointer mode.
- ///
- bool isPPC64() const;
-
// useSoftFloat - Return true if soft-float option is turned on.
bool useSoftFloat() const {
if (isAIXABI() && !HasHardFloat)
diff --git a/llvm/test/CodeGen/PowerPC/i64_fp.ll b/llvm/test/CodeGen/PowerPC/i64_fp.ll
index b9456150df7b8..3cec87d6653a6 100644
--- a/llvm/test/CodeGen/PowerPC/i64_fp.ll
+++ b/llvm/test/CodeGen/PowerPC/i64_fp.ll
@@ -1,17 +1,17 @@
; fcfid and fctid should be generated when the 64bit feature is enabled, but not
; otherwise.
-; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=+64bit | \
+; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=+64bit-support | \
; RUN: grep fcfid
-; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=+64bit | \
+; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=+64bit-support | \
; RUN: grep fctidz
; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mcpu=g5 | \
; RUN: grep fcfid
; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mcpu=g5 | \
; RUN: grep fctidz
-; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=-64bit | \
+; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=-64bit-support | \
; RUN: not grep fcfid
-; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=-64bit | \
+; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mattr=-64bit-support | \
; RUN: not grep fctidz
; RUN: llc -verify-machineinstrs < %s -mattr=-vsx -mtriple=ppc32-- -mcpu=g4 | \
; RUN: not grep fcfid
|
The way this was previously structured does not allow access to the predicates inside of PPCRegisterInfo
This was being used for 2 different purposes.
The TargetMachine constructor prepends +64bit based on isPPC64
triples as a mode switch. The same feature name was also explicitly
added to different processors, making it impossible to perform a pure
feature check for whether 64-bit mode is enabled ir not. i.e.,
checkFeatures("+64bit") would be true even for ppc32 triples.
The comment in tablegen suggests it's relevant to track which processors
support 64-bit mode independently of whether that's the active compile
target, so replace that with a new feature.
fe36dae to
b5ab7a9
Compare
f86faf6 to
0d4d26a
Compare
lei137
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable to me. Thx for the improvement!
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/13547 Here is the relevant piece of the build log for the reference |
|
I have bisected a crash that folks have seen in the Linux kernel to this change. A C reproducer from enum { false } typedef bool;
typedef int size_t;
struct vgetrandom_state {
long generation;
} __arch_chacha20_blocks_nostack(unsigned *);
void *__cvdso_getrandom_data_buffer, *__cvdso_getrandom_data_opaque_state;
unsigned __cvdso_getrandom_data_counter[1];
size_t __cvdso_getrandom_data_len;
void getrandom_syscall(void *, size_t);
int __cvdso_getrandom_data() {
struct vgetrandom_state *state = __cvdso_getrandom_data_opaque_state;
size_t batch_len, orig_len = __cvdso_getrandom_data_len;
bool in_use;
void *orig_buffer = __cvdso_getrandom_data_buffer;
long current_generation;
in_use = 0;
if (__builtin_expect(in_use, 0))
goto fallback_syscall;
retry_generation:
current_generation = 0;
if (__builtin_expect(state->generation != current_generation, 0))
more_batch:
batch_len = 0;
if (batch_len)
goto retry_generation;
__arch_chacha20_blocks_nostack(__cvdso_getrandom_data_counter);
goto more_batch;
fallback_syscall:
getrandom_syscall(orig_buffer, orig_len);
return __cvdso_getrandom_data_len;
}
target datalayout = "E-m:e-p:32:32-Fn32-i64:64-n32"
target triple = "powerpc-unknown-linux-gnu"
define i32 @__cvdso_getrandom_data() #0 {
entry:
br label %if.end4
if.end4: ; preds = %if.end4, %entry
call void null(ptr null, ptr null)
br label %if.end4
}
attributes #0 = { "target-cpu"="e5500" } |
Fix in #159893 |

This was being used for 2 different purposes.
The TargetMachine constructor prepends +64bit based on isPPC64
triples as a mode switch. The same feature name was also explicitly
added to different processors, making it impossible to perform a pure
feature check for whether 64-bit mode is enabled ir not. i.e.,
checkFeatures("+64bit") would be true even for ppc32 triples.
The comment in tablegen suggests it's relevant to track which processors
support 64-bit mode independently of whether that's the active compile
target, so replace that with a new feature.