[PATCH] D15075: No error for conflict between inputs\outputs and clobber list

Akira Hatanaka via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 09:13:20 PDT 2016


ahatanak added a comment.

In https://reviews.llvm.org/D15075#486986, @zizhar wrote:

> Akira,
>  You've mentioned a good point, this X86 logic should indeed be moved to X86TargetInfo.
>  The current convertConstraint() implementation is not doing what I need – it doesn’t handle all the input/output constraints I need, and it returns the constraint in a different format than the one I need.
>  E.g.  convertConstraint() does not handle “r” constraints or special characters like “=”, “+”, also, the function returns the registers as “{ax}”, when I need the “ax”.
>  I think it is better to add a new function for my logic instead of trying to adjust this function to handle both its old logic and my new logic.
>
> My new solution is going to be to create another virtual function in TargetInfo that will do everything I need.
>  The code that is currently under GetConstraintRegister() and ExtractRegisterName() will now be part of the X86TargetInfo implementation of this virtual function.
>  For all the other architectures’ TargetInfos I’ll create a default implementation that will return an empty string and they can implement it if they want to (until they do, the existing behavior will be retained). 
>  Can I have your opinion on this? Do you see a better way to implement this?


I agree with you. You can define a virtual function in TargetInfo, which checks conflicts between clobbers and input/output, and override it in X86's TargetInfo.


================
Comment at: lib/Headers/Intrin.h:841-874
@@ -840,44 +840,36 @@
 #if defined(__i386__) || defined(__x86_64__)
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsb(unsigned char *__dst, unsigned char const *__src, size_t __n) {
-  __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n)
-                        : "%edi", "%esi", "%ecx");
+  __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsd(unsigned long *__dst, unsigned long const *__src, size_t __n) {
-  __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n)
-                        : "%edi", "%esi", "%ecx");
+  __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsw(unsigned short *__dst, unsigned short const *__src, size_t __n) {
-  __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n)
-                        : "%edi", "%esi", "%ecx");
+  __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosb(unsigned char *__dst, unsigned char __x, size_t __n) {
-  __asm__("rep stosb" : : "D"(__dst), "a"(__x), "c"(__n)
-                        : "%edi", "%ecx");
+  __asm__("rep stosb" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosd(unsigned long *__dst, unsigned long __x, size_t __n) {
-  __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n)
-                        : "%edi", "%ecx");
+  __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosw(unsigned short *__dst, unsigned short __x, size_t __n) {
-  __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n)
-                        : "%edi", "%ecx");
+  __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 #endif
 #ifdef __x86_64__
 static __inline__ void __DEFAULT_FN_ATTRS
 __movsq(unsigned long long *__dst, unsigned long long const *__src, size_t __n) {
-  __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n)
-                        : "%edi", "%esi", "%ecx");
+  __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n) :);
 }
 static __inline__ void __DEFAULT_FN_ATTRS
 __stosq(unsigned __int64 *__dst, unsigned __int64 __x, size_t __n) {
-  __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n)
-                        : "%edi", "%ecx");
+  __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n) :);
 }
 #endif
----------------
majnemer wrote:
> The clobber should be "memory", no?
> Er, and shouldn't the we list `__dst`, `__x` and `__n` as outputs because they are modified?
Yes, I think we need "memory" here because it writes to the memory pointed to by dst.

Do we need to add x to the output too? I think dst and n are modfied by stosq and rep (which I think is why %edi and %ecx were added to the clobber list), but I don't see how x gets modified.


https://reviews.llvm.org/D15075





More information about the cfe-commits mailing list