- Notifications
You must be signed in to change notification settings - Fork14.5k
[GlobalISel] Allow Legalizer to lower volatile memcpy family.#145997
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
petechou wants to merge2 commits intollvm:mainChoose a base branch
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
+187 −126
Conversation
This file contains hidden or 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
This change updates legalizer to allow lowering volatile memcpy familyas a target might rely on lowering to legalize them. Also, legalizeralready lowers volatile G_MEMCPY_INLINE and has the capability to lowermemcpy family. For targets like aarch64 use legalizer to lower memcpyfamily as a combiner optimization, the change adds an additionalargument for lowering to skip volatile and keep the existing behavior inthat case.
Member
llvmbot commentedJun 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-amdgpu Author: Pete Chou (petechou) ChangesThis change updates legalizer to allow lowering volatile memcpy family Full diff:https://github.com/llvm/llvm-project/pull/145997.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.hindex ea0873f41ebba..be8169c79f219 100644--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h@@ -489,7 +489,8 @@ class LegalizerHelper { LLVM_ABI LegalizeResult lowerVectorReduction(MachineInstr &MI); LLVM_ABI LegalizeResult lowerMemcpyInline(MachineInstr &MI); LLVM_ABI LegalizeResult lowerMemCpyFamily(MachineInstr &MI,- unsigned MaxLen = 0);+ unsigned MaxLen = 0,+ bool SkipVolatile = false); LLVM_ABI LegalizeResult lowerVAArg(MachineInstr &MI); };diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cppindex b1e851183de0d..9e203ce863639 100644--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp@@ -1704,7 +1704,7 @@ bool CombinerHelper::tryCombineMemCpyFamily(MachineInstr &MI, MachineIRBuilder HelperBuilder(MI); GISelObserverWrapper DummyObserver; LegalizerHelper Helper(HelperBuilder.getMF(), DummyObserver, HelperBuilder);- return Helper.lowerMemCpyFamily(MI, MaxLen) ==+ return Helper.lowerMemCpyFamily(MI, MaxLen, /*SkipVolatile=*/true) == LegalizerHelper::LegalizeResult::Legalized; }diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cppindex b87b029d01632..b5af3c64d50fa 100644--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp@@ -10066,7 +10066,8 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src, } LegalizerHelper::LegalizeResult-LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen) {+LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen,+ bool SkipVolatile) { const unsigned Opc = MI.getOpcode(); // This combine is fairly complex so it's not written with a separate // matcher function.@@ -10099,8 +10100,8 @@ LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen) { } bool IsVolatile = MemOp->isVolatile();- // Don't try to optimize volatile.- if (IsVolatile)+ // Don't try to optimize volatile when not allowed.+ if (SkipVolatile && IsVolatile) return UnableToLegalize; if (MaxLen && KnownLen > MaxLen)diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mirindex be3fe91407fdf..4f5f52b25cdf7 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memcpy_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memcpy_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMCPY %2:_(p0), %5:_(p0), %7:_(s64), 0 :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mirindex a82ca30209820..0392aef6fe030 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memcpyinline_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memcpyinline_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMCPY_INLINE %2:_(p0), %5:_(p0), %7:_(s64) :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mirindex e7cfaab135beb..1f8d1aac24ebb 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memmove_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memmove_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMMOVE %2:_(p0), %5:_(p0), %7:_(s64), 0 :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mirindex 021cebbb6cb49..dda94e1550585 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir@@ -30,3 +30,32 @@ body: | S_ENDPGM 0 ...+---+name: memset_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2++ ; CHECK-LABEL: name: memset_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[COPY2]](s32)+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s8) = COPY [[TRUNC]](s8)+ ; CHECK-NEXT: G_STORE [[COPY2]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s16) = G_TRUNC %3:_(s32)+ %5:_(s8) = G_TRUNC %4:_(s16)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMSET %2:_(p0), %5:_(s8), %7:_(s64), 0 :: (volatile store (s8))+ S_ENDPGM 0++... |
@llvm/pr-subscribers-llvm-globalisel Author: Pete Chou (petechou) ChangesThis change updates legalizer to allow lowering volatile memcpy family Full diff:https://github.com/llvm/llvm-project/pull/145997.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.hindex ea0873f41ebba..be8169c79f219 100644--- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h+++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h@@ -489,7 +489,8 @@ class LegalizerHelper { LLVM_ABI LegalizeResult lowerVectorReduction(MachineInstr &MI); LLVM_ABI LegalizeResult lowerMemcpyInline(MachineInstr &MI); LLVM_ABI LegalizeResult lowerMemCpyFamily(MachineInstr &MI,- unsigned MaxLen = 0);+ unsigned MaxLen = 0,+ bool SkipVolatile = false); LLVM_ABI LegalizeResult lowerVAArg(MachineInstr &MI); };diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cppindex b1e851183de0d..9e203ce863639 100644--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp@@ -1704,7 +1704,7 @@ bool CombinerHelper::tryCombineMemCpyFamily(MachineInstr &MI, MachineIRBuilder HelperBuilder(MI); GISelObserverWrapper DummyObserver; LegalizerHelper Helper(HelperBuilder.getMF(), DummyObserver, HelperBuilder);- return Helper.lowerMemCpyFamily(MI, MaxLen) ==+ return Helper.lowerMemCpyFamily(MI, MaxLen, /*SkipVolatile=*/true) == LegalizerHelper::LegalizeResult::Legalized; }diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cppindex b87b029d01632..b5af3c64d50fa 100644--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp@@ -10066,7 +10066,8 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src, } LegalizerHelper::LegalizeResult-LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen) {+LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen,+ bool SkipVolatile) { const unsigned Opc = MI.getOpcode(); // This combine is fairly complex so it's not written with a separate // matcher function.@@ -10099,8 +10100,8 @@ LegalizerHelper::lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen) { } bool IsVolatile = MemOp->isVolatile();- // Don't try to optimize volatile.- if (IsVolatile)+ // Don't try to optimize volatile when not allowed.+ if (SkipVolatile && IsVolatile) return UnableToLegalize; if (MaxLen && KnownLen > MaxLen)diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mirindex be3fe91407fdf..4f5f52b25cdf7 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpy.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memcpy_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memcpy_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMCPY %2:_(p0), %5:_(p0), %7:_(s64), 0 :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mirindex a82ca30209820..0392aef6fe030 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memcpyinline.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memcpyinline_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memcpyinline_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMCPY_INLINE %2:_(p0), %5:_(p0), %7:_(s64) :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mirindex e7cfaab135beb..1f8d1aac24ebb 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memmove.mir@@ -31,3 +31,33 @@ body: | S_ENDPGM 0 ...+---+name: memmove_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3++ ; CHECK-LABEL: name: memmove_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2, $vgpr3+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $vgpr3+ ; CHECK-NEXT: [[MV1:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY2]](s32), [[COPY3]](s32)+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[MV1]](p0) :: (volatile load (s8))+ ; CHECK-NEXT: G_STORE [[LOAD]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s32) = COPY $vgpr3+ %5:_(p0) = G_MERGE_VALUES %3:_(s32), %4:_(s32)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMMOVE %2:_(p0), %5:_(p0), %7:_(s64), 0 :: (volatile store (s8)), (volatile load (s8))+ S_ENDPGM 0++...diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mirindex 021cebbb6cb49..dda94e1550585 100644--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-memset.mir@@ -30,3 +30,32 @@ body: | S_ENDPGM 0 ...+---+name: memset_test_volatile+body: |+ bb.0:+ liveins: $vgpr0, $vgpr1, $vgpr2++ ; CHECK-LABEL: name: memset_test_volatile+ ; CHECK: liveins: $vgpr0, $vgpr1, $vgpr2+ ; CHECK-NEXT: {{ $}}+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1+ ; CHECK-NEXT: [[MV:%[0-9]+]]:_(p0) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $vgpr2+ ; CHECK-NEXT: [[TRUNC:%[0-9]+]]:_(s8) = G_TRUNC [[COPY2]](s32)+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:_(s8) = COPY [[TRUNC]](s8)+ ; CHECK-NEXT: G_STORE [[COPY2]](s32), [[MV]](p0) :: (volatile store (s8))+ ; CHECK-NEXT: S_ENDPGM 0+ %0:_(s32) = COPY $vgpr0+ %1:_(s32) = COPY $vgpr1+ %2:_(p0) = G_MERGE_VALUES %0:_(s32), %1:_(s32)+ %3:_(s32) = COPY $vgpr2+ %4:_(s16) = G_TRUNC %3:_(s32)+ %5:_(s8) = G_TRUNC %4:_(s16)+ %6:_(s32) = G_CONSTANT i32 1+ %7:_(s64) = G_ZEXT %6:_(s32)+ G_MEMSET %2:_(p0), %5:_(s8), %7:_(s64), 0 :: (volatile store (s8))+ S_ENDPGM 0++... |
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.
Please help review the pr and merge it if it looks good. Thanks.
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.
For AMDGPU, SelectionDAG can handle volatile memcpy but GlobalISEL would fail to legalize it, so it seems to me GlobalISEL should allow lowering volatile memcpy family.
| // Don't try to optimize volatile. | ||
| if (IsVolatile) | ||
| // Don't try to optimize volatile when not allowed. | ||
| if (SkipVolatile &&IsVolatile) |
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 is not an optimization and this should not be skipped, nor be an option. I would just delete the check altogether
ContributorAuthor
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.
ISTM, when the code was first introduced in13af1ed, it's part of combiner code and used as an optimization for AArch64 to inline memcpy sequence instead of creating libcall. In a later refactoring change36527cb, the code was moved to legalizer to allow AMDGPU legalizing memcpy family of intrinsics by lowering them. However, the code is still used bycombiner, and the logic to skip volatile was kept for combiner users. So, that's why I think it's used by some targets as an optimization.
The change tries to make the least update to support both legalizer and combiner users. Another alternative could be deleting the check as you suggested, and do the check in combiner before calling legalizer lowering function instead. That might add some duplicate code like checking zero length and volatile though. What do you think?
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.
BTW, in36527cb theMaxLen argument of the added LegalizerHelper lowerMemCpyFamily function could be a defualt argument.
LegalizeResult lowerMemCpyFamily(MachineInstr &MI, unsigned MaxLen = 0);
It's probably for the same reason to support both legalizer and combiner users, I guess.
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.
@arsenm Could you please provide your recommendations? I would appreciate your input. Thanks.
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.
@arsenm Ping. Thanks.
@mbrkusanin This pr extends36527cb to support legalizing volatile memcpy family. Appreciate it if you could also provide comments here. Thanks.
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.
@arsenm, ping^3.
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.
I think we should always be lowering volatile memcpy operations if possible so the check can be deleted.
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.
aemerson approved these changesJul 19, 2025
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.
LGTM. This wasn't your doing but it's strange why the original commit that added the SDAG check lines to the mops test didn't use the same optimization level as GISel (O2 vs O3):5aa08bf
Sign up for freeto 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.
Uh oh!
There was an error while loading.Please reload this page.
This change updates legalizer to allow lowering volatile memcpy family
as a target might rely on lowering to legalize them.