[PowerPC] clean unused PPC target feature FeatureBPERMD

[diggerlin]

clean unused PPC target feature FeatureBPERMD.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

Author: zhijian lin (diggerlin)

Changes

clean unused PPC target feature FeatureBPERMD.

---

Full diff: https://github.com/llvm/llvm-project/pull/159782.diff

1 Files Affected:

• (modified) llvm/lib/Target/PowerPC/PPC.td (+1-4)

diff --git a/llvm/lib/Target/PowerPC/PPC.td b/llvm/lib/Target/PowerPC/PPC.td
index 386d0f65d1ed1..327ca192b1c5b 100644
--- a/llvm/lib/Target/PowerPC/PPC.td
+++ b/llvm/lib/Target/PowerPC/PPC.td
@@ -129,8 +129,6 @@ def FeatureFPCVT     : SubtargetFeature<"fpcvt", "HasFPCVT", "true",
                                         [FeatureFPU]>;
 def FeatureISEL      : SubtargetFeature<"isel","HasISEL", "true",
                                         "Enable the isel instruction">;
-def FeatureBPERMD    : SubtargetFeature<"bpermd", "HasBPERMD", "true",
-                                        "Enable the bpermd instruction">;
 def FeatureExtDiv    : SubtargetFeature<"extdiv", "HasExtDiv", "true",
                                         "Enable extended divide instructions">;
 def FeatureLDBRX     : SubtargetFeature<"ldbrx","HasLDBRX", "true",
@@ -377,7 +375,7 @@ def NoNaNsFPMath
     : Predicate<"Subtarget->getTargetMachine().Options.NoNaNsFPMath">;
 def NaNsFPMath
     : Predicate<"!Subtarget->getTargetMachine().Options.NoNaNsFPMath">;
-def HasBPERMD : Predicate<"Subtarget->hasBPERMD()">;
+def HasBPERMD : Predicate<"Subtarget->getCPUDirective() >= PPC::DIR_PWR7">;
 def HasExtDiv : Predicate<"Subtarget->hasExtDiv()">;
 def IsISA2_06 : Predicate<"Subtarget->isISA2_06()">;
 def IsISA2_07 : Predicate<"Subtarget->isISA2_07()">;
@@ -436,7 +434,6 @@ def ProcessorFeatures {
                                                   FeatureLDBRX,
                                                   Feature64BitSupport,
                                                   /* Feature64BitRegs, */
-                                                  FeatureBPERMD,
                                                   FeatureExtDiv,
                                                   FeatureMFTB,
                                                   DeprecatedDST,

[s-barannikov]

Should it instead add AssemblerPredicate to make sure the instructions are not assembled/disassembled without the feature enabled?

[diggerlin]

since the HasBPERMD defined in

def FeatureBPERMD    : SubtargetFeature<"bpermd", "HasBPERMD", "true",
                                        "Enable the bpermd instruction">;

is not used in the llvm source code. we can delete it. the patch only clean FeatureBPERMD, which is NFC patch. Adding AssemblerPredicate change the purpose of the patch(not a NFC). if we want to add it , I can create a separate patch for it?

[nico]

Is this check-clang failure http://45.33.8.238/linux/178016/step_6.txt due to this?

[s-barannikov]

It broke the CI as well.
I'm not sure I understand why the CI passed on this PR. It is not the first time I see that CI passes on a PR but starts failing after the PR is merged.

[s-barannikov]

I'm not sure I understand why the CI passed on this PR

Or did it not? The checks were red and became green after updating a branch, but the merge update didn't trigger all checks

[boomanaiden154]

Or did it not? The checks were red and became green after updating a branch, but the merge update didn't trigger all checks

They're definitely showing up as red for me at the time of merge.

[s-barannikov]

They're definitely showing up as red for me at the time of merge.

That was before the merge commit, it seems. Merge commit didn't seem to re-trigger the failing jobs and CI passed:

[s-barannikov]

@boomanaiden154 I believe the picture you posted shows checks that were executed after the PR was merged.

[boomanaiden154]

I believe the picture you posted shows checks that were executed after the PR was merged.

We don't run premerge CI after merge and have it explicitly setup to cancel workflows upon merge.

Shows the CI checks failing on the merge commit for me. The Github UI is being a bit finnicky about the commit statuses though.

[s-barannikov]

Then I'm confused. What are those skipped checks near the merge commit that I posted above? Is that just github showing them incorrectly (very weird)?
I just pressed the update button on one of my PRs and it did re-trigger the required Linux/Windows builds...

[s-barannikov]

OK I think I understood.

There were three CI runs:

1. On PR creation (failed)
2. After pushing the update button (failed again)
3. After merging the PR. The checks were skipped, and it is this status that I'm seeing. For some reason it is attached to the updating merge commit.

This helped clarify what happened. It indeed looks like the PR was merged with CI checks red.