Skip to content

[X86] Support EVEX compression from MOVBErr to BSWAP #79775

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

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

XinWang10
Copy link
Contributor

@XinWang10 XinWang10 commented Jan 29, 2024

APX promoted MOVBE instructions were supported in #77431. The reg2reg variants of MOVBE are newly introduced by APX and can be optimized to BSWAP instruction when the 2 register operands are same.

This patch adds manual entries for MOVBErr instructions when we do ndd to non-ndd compression #77731.
RFC: https://discourse.llvm.org/t/rfc-design-for-apx-feature-egpr-and-ndd-support/73031/4

@XinWang10 XinWang10 requested review from KanRobert, phoebewang and RKSimon and removed request for KanRobert January 29, 2024 02:30
@llvmbot
Copy link
Member

llvmbot commented Jan 29, 2024

@llvm/pr-subscribers-backend-x86

Author: None (XinWang10)

Changes

APX promoted MOVBE instructions have been supported in #77431.
APX introduce 2 new forms MOVBE*rr instructions which only have MOVBErm/mr before. The new MOVBE*rr could be optimized to BSWAP*r instruction when the 2 operands are same registers.
This patch add entries for MOVBE*rr instructions when we do ndd to non-ndd compression(#77731)
RFC: https://discourse.llvm.org/t/rfc-design-for-apx-feature-egpr-and-ndd-support/73031/4


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

3 Files Affected:

  • (modified) llvm/lib/Target/X86/X86CompressEVEX.cpp (+5-2)
  • (modified) llvm/test/CodeGen/X86/apx/compress-evex.mir (+10)
  • (modified) llvm/utils/TableGen/X86ManualCompressEVEXTables.def (+2)
diff --git a/llvm/lib/Target/X86/X86CompressEVEX.cpp b/llvm/lib/Target/X86/X86CompressEVEX.cpp
index a9704e30478d136..7c018f54b7e0a75 100644
--- a/llvm/lib/Target/X86/X86CompressEVEX.cpp
+++ b/llvm/lib/Target/X86/X86CompressEVEX.cpp
@@ -225,8 +225,11 @@ static bool CompressEVEXImpl(MachineInstr &MI, const X86Subtarget &ST) {
   //
   // For AVX512 cases, EVEX prefix is needed in order to carry this information
   // thus preventing the transformation to VEX encoding.
-  bool IsND = X86II::hasNewDataDest(TSFlags);
-  if (TSFlags & X86II::EVEX_B)
+  // MOVBE*rr is special because it has sematic of NDD but not set EVEX_B.
+  bool IsMovberr =
+      MI.getOpcode() == X86::MOVBE32rr || MI.getOpcode() == X86::MOVBE64rr;
+  bool IsND = X86II::hasNewDataDest(TSFlags) || IsMovberr;
+  if (TSFlags & X86II::EVEX_B || IsMovberr)
     if (!IsND || !isRedundantNewDataDest(MI, ST))
       return false;
 
diff --git a/llvm/test/CodeGen/X86/apx/compress-evex.mir b/llvm/test/CodeGen/X86/apx/compress-evex.mir
index 5a3d7ceb10c432e..997a8395aa752fd 100644
--- a/llvm/test/CodeGen/X86/apx/compress-evex.mir
+++ b/llvm/test/CodeGen/X86/apx/compress-evex.mir
@@ -71,3 +71,13 @@ body:             |
     renamable $rax = XOR64rr_NF_ND killed renamable $rax, killed renamable $r16
     RET64 $rax
 ...
+---
+name:            bswapr_to_movberr
+body:             |
+  bb.0.entry:
+    liveins: $rax
+    ; CHECK: bswapq  %rax                            # EVEX TO LEGACY Compression encoding: [0x48,0x0f,0xc8]
+    renamable $rax = MOVBE64rr killed renamable $rax
+    RET64 killed $rax
+
+...
diff --git a/llvm/utils/TableGen/X86ManualCompressEVEXTables.def b/llvm/utils/TableGen/X86ManualCompressEVEXTables.def
index 58ca10e9e10f8df..77cf65be6842566 100644
--- a/llvm/utils/TableGen/X86ManualCompressEVEXTables.def
+++ b/llvm/utils/TableGen/X86ManualCompressEVEXTables.def
@@ -328,4 +328,6 @@ ENTRY(VBROADCASTSDZ256rm, VBROADCASTSDYrm)
 ENTRY(VBROADCASTSDZ256rr, VBROADCASTSDYrr)
 ENTRY(VPBROADCASTQZ256rm, VPBROADCASTQYrm)
 ENTRY(VPBROADCASTQZ256rr, VPBROADCASTQYrr)
+ENTRY(MOVBE32rr, BSWAP32r)
+ENTRY(MOVBE64rr, BSWAP64r)
 #undef ENTRY

Copy link
Contributor

@KanRobert KanRobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one comment

@KanRobert KanRobert changed the title [X86] Support encoding compress from movberr to bwap [X86] Support EVEX compression from MOVBErr to BSWAP Jan 29, 2024
@XinWang10 XinWang10 merged commit 1a219e9 into llvm:main Jan 30, 2024
@XinWang10
Copy link
Contributor Author

Thanks~

@RKSimon
Copy link
Collaborator

RKSimon commented Jan 30, 2024

@XinWang10 I'm seeing an expensive checks error that I think is due to this commit:

# After Compressing EVEX instrs when possible
# Machine code for function bswapr_to_movberr: IsSSA, NoPHIs, NoVRegs

bb.0.entry:
  renamable $rax = BSWAP64r killed renamable $rax
  RET64 killed $rax

# End machine code for function bswapr_to_movberr.

*** Bad machine code: Operand should be tied ***
- function:    bswapr_to_movberr
- basic block: %bb.0 entry (0x1ba33239778)
- instruction: renamable $rax = BSWAP64r killed renamable $rax
- operand 1:   killed renamable $rax
LLVM ERROR: Found 1 machine code errors.

@KanRobert
Copy link
Contributor

KanRobert commented Jan 30, 2024

@RKSimon I think he addressed you comment incompletely after I accepted the PR

  if (IsND)
    MI.tieOperands(0, 1);

He forgot to check MOVBE for this.

RKSimon added a commit that referenced this pull request Jan 30, 2024
…tructions

Minor correction for #79775 - noticed in EXPENSIVE_CHECKS builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants