This is an archive of the discontinued LLVM Phabricator instance.

Differential D106210

[MS] Preserve base register %esi around movs[bwl]
ClosedPublic

Authored by namazso on Jul 16 2021, 9:14 PM.

Details

Reviewers
echristo
rnk
craig.topper
Commits
rG91bc85b1ebaa: [MS] Preserve base register %esi around movs[bwl]
Summary

fix for behavior reported in https://bugs.llvm.org/show_bug.cgi?id=51100 workaround for root cause https://bugs.llvm.org/show_bug.cgi?id=16830

similar to https://reviews.llvm.org/D101338

Diff Detail

Unit TestsFailed

TimeTest
200 msx64 debian > Clang.CodeGen::ms-intrinsics.c
2,550 msx64 debian > libarcher.barrier::barrier.c
2,890 msx64 debian > libarcher.critical::critical.c
2,800 msx64 debian > libarcher.critical::lock-nested.c
2,680 msx64 debian > libarcher.parallel::parallel-simple2.c
View Full Test Results (20 Failed)

Event Timeline

namazso requested review of this revision.Jul 16 2021, 9:14 PM
namazso created this revision.
Herald added a subscriber: cfe-commits. · View Herald TranscriptJul 16 2021, 9:14 PM
namazso edited the summary of this revision. (Show Details)Jul 16 2021, 9:16 PM
Harbormaster completed remote builds in B114660: Diff 359526.Jul 16 2021, 9:53 PM
namazso updated this revision to Diff 359550.Jul 17 2021, 1:45 AM

fix linter warnings, update tests

Harbormaster completed remote builds in B114674: Diff 359550.Jul 17 2021, 2:22 AM
namazso added reviewers: echristo, rnk.Jul 17 2021, 2:32 AM
craig.topper accepted this revision.Jul 17 2021, 11:32 AM
craig.topper added a subscriber: craig.topper.

LGTM

This revision is now accepted and ready to land.Jul 17 2021, 11:32 AM
namazso added a comment.Jul 17 2021, 12:08 PM

I don't have commit access, could someone commit this for me? Thanks in advance.

craig.topper added a comment.Jul 18 2021, 1:35 PM
In D106210#2885538, @namazso wrote:

I don't have commit access, could someone commit this for me? Thanks in advance.

I can commit it. How would you like your name to appear in the author line of the git log?

namazso added a comment.Jul 18 2021, 2:48 PM

i'd prefer it as just namazso with email admin@namazso.eu

This revision was landed with ongoing or failed builds.Jul 23 2021, 1:28 AM
Closed by commit rG91bc85b1ebaa: [MS] Preserve base register %esi around movs[bwl] (authored by namazso, committed by Wang, Pengfei <pengfei.wang@intel.com>). · Explain Why
This revision was automatically updated to reflect the committed changes.
pengfei added a commit: rG91bc85b1ebaa: [MS] Preserve base register %esi around movs[bwl].
pengfei added a subscriber: pengfei.Jul 23 2021, 1:30 AM

Happened to see it. I thought Craig might miss it.

craig.topper added a comment.Jul 23 2021, 10:16 AM
In D106210#2899361, @pengfei wrote:

Happened to see it. I thought Craig might miss it.

Thanks @pengfei

Revision Contents

PathSize
clang/
lib/
Headers/
27 lines

Diff 359526

clang/lib/Headers/intrin.h

Context not available.
static __inline__ void __DEFAULT_FN_ATTRS __movsb(unsigned char *__dst, static __inline__ void __DEFAULT_FN_ATTRS __movsb(unsigned char *__dst,
unsigned char const *__src, unsigned char const *__src,
size_t __n) { size_t __n) {
#if defined(__x86_64__)
__asm__ __volatile__("rep movsb" : "+D"(__dst), "+S"(__src), "+c"(__n) __asm__ __volatile__("rep movsb" : "+D"(__dst), "+S"(__src), "+c"(__n)
: : "memory"); : : "memory");
#else
__asm__ __volatile__("xchg %%esi, %1\nrep movsb\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  __asm__ __volatile__("xchg %%esi, %1\nrep movsb\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
-                       : : "memory");
+  __asm__ __volatile__("xchg %%esi, %1\nrep movsb\nxchg %%esi, %1"
+                       : "+D"(__dst), "+r"(__src), "+c"(__n)
+                       :
+                       : "memory");
Lint: Pre-merge checks: clang-format: please reformat the code ``` - __asm__ __volatile__("xchg %%esi, %1\nrep…
: : "memory");
#endif
} }
static __inline__ void __DEFAULT_FN_ATTRS __movsd(unsigned long *__dst, static __inline__ void __DEFAULT_FN_ATTRS __movsd(unsigned long *__dst,
unsigned long const *__src, unsigned long const *__src,
size_t __n) { size_t __n) {
__asm__ __volatile__("rep movsl" #if defined(__x86_64__)
: "+D"(__dst), "+S"(__src), "+c"(__n) __asm__ __volatile__("rep movsl" : "+D"(__dst), "+S"(__src), "+c"(__n)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  __asm__ __volatile__("rep movsl" : "+D"(__dst), "+S"(__src), "+c"(__n)
-                       : : "memory");
+  __asm__ __volatile__("rep movsl"
+                       : "+D"(__dst), "+S"(__src), "+c"(__n)
+                       :
+                       : "memory");
Lint: Pre-merge checks: clang-format: please reformat the code ``` - __asm__ __volatile__("rep movsl" : "+D"(__dst)…
: : : "memory");
: "memory"); #else
__asm__ __volatile__("xchg %%esi, %1\nrep movsl\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  __asm__ __volatile__("xchg %%esi, %1\nrep movsl\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
-                       : : "memory");
+  __asm__ __volatile__("xchg %%esi, %1\nrep movsl\nxchg %%esi, %1"
+                       : "+D"(__dst), "+r"(__src), "+c"(__n)
+                       :
+                       : "memory");
Lint: Pre-merge checks: clang-format: please reformat the code ``` - __asm__ __volatile__("xchg %%esi, %1\nrep…
: : "memory");
#endif
} }
static __inline__ void __DEFAULT_FN_ATTRS __movsw(unsigned short *__dst, static __inline__ void __DEFAULT_FN_ATTRS __movsw(unsigned short *__dst,
unsigned short const *__src, unsigned short const *__src,
size_t __n) { size_t __n) {
__asm__ __volatile__("rep movsw" #if defined(__x86_64__)
: "+D"(__dst), "+S"(__src), "+c"(__n) __asm__ __volatile__("rep movsw" : "+D"(__dst), "+S"(__src), "+c"(__n)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  __asm__ __volatile__("rep movsw" : "+D"(__dst), "+S"(__src), "+c"(__n)
-                       : : "memory");
+  __asm__ __volatile__("rep movsw"
+                       : "+D"(__dst), "+S"(__src), "+c"(__n)
+                       :
+                       : "memory");
Lint: Pre-merge checks: clang-format: please reformat the code ``` - __asm__ __volatile__("rep movsw" : "+D"(__dst)…
: : : "memory");
: "memory"); #else
__asm__ __volatile__("xchg %%esi, %1\nrep movsw\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  __asm__ __volatile__("xchg %%esi, %1\nrep movsw\nxchg %%esi, %1" : "+D"(__dst), "+r"(__src), "+c"(__n)
-                       : : "memory");
+  __asm__ __volatile__("xchg %%esi, %1\nrep movsw\nxchg %%esi, %1"
+                       : "+D"(__dst), "+r"(__src), "+c"(__n)
+                       :
+                       : "memory");
Lint: Pre-merge checks: clang-format: please reformat the code ``` - __asm__ __volatile__("xchg %%esi, %1\nrep…
: : "memory");
#endif
} }
static __inline__ void __DEFAULT_FN_ATTRS __stosd(unsigned long *__dst, static __inline__ void __DEFAULT_FN_ATTRS __stosd(unsigned long *__dst,
unsigned long __x, unsigned long __x,
Context not available.