llvm dev - Nov 2013 - [LLVMdev] [PATCH] Do not generate nopl instruction on CPUs that don't support it.

Hi

This patch fixes code generation bug - 586-class CPUs don't support the 
nopl instruction and some 686-class CPUs don't support it too.

I created bug 17792 for that.


BTW. I think you should also optimize padding on these CPUs - instead of a 
stream of 0x90 nops, you should generate variants of "lea (%esi),
%esi"
instruction like gcc.



This patch disables generation of nopl instructions on the following CPUs:

i686 - there are i686-class CPUs that don't have nopl: Via c3, Transmeta
	Crusoe, Microsoft VirtualBox - see
	https://bbs.archlinux.org/viewtopic.php?pid=775414
k6, k6-2, k6-3, winchip-c6, winchip2 - these are 586-class CPUs
via c3 c3-2 - see https://bugs.archlinux.org/task/19733 as a proof that
	Via c3 and c3-Nehemiah don't have nopl

Mikulas

---
 lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
==================================================================---
llvm.orig/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp	2013-11-03
00:24:20.000000000 +0100
+++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp	2013-11-03
20:57:14.000000000 +0100
@@ -309,7 +309,10 @@ bool X86AsmBackend::writeNopData(uint64_
   // This CPU doesnt support long nops. If needed add more.
   // FIXME: Can we get this from the subtarget somehow?
   if (CPU == "generic" || CPU == "i386" || CPU ==
"i486" || CPU == "i586" ||
-      CPU == "pentium" || CPU == "pentium-mmx" || CPU ==
"geode") {
+      CPU == "pentium" || CPU == "pentium-mmx" || CPU ==
"i686" ||
+      CPU == "k6" || CPU == "k6-2" || CPU ==
"k6-3" || CPU == "geode" ||
+      CPU == "winchip-c6" || CPU == "winchip2" || CPU ==
"c3" ||
+      CPU == "c3-2") {
     for (uint64_t i = 0; i < Count; ++i)
       OW->Write8(0x90);
     return true;

Please include a testcase with the patch.

gas uses " nopl   0x0(%eax)" for k6_2. Are you sure it is a gas bug?

On 3 November 2013 13:50, Mikulas Patocka
<mikulas at artax.karlin.mff.cuni.cz> wrote:
> Hi
>
> This patch fixes code generation bug - 586-class CPUs don't support the
> nopl instruction and some 686-class CPUs don't support it too.
>
> I created bug 17792 for that.
>
>
> BTW. I think you should also optimize padding on these CPUs - instead of a
> stream of 0x90 nops, you should generate variants of "lea (%esi),
%esi"
> instruction like gcc.
>
>
>
> This patch disables generation of nopl instructions on the following CPUs:
>
> i686 - there are i686-class CPUs that don't have nopl: Via c3,
Transmeta
>         Crusoe, Microsoft VirtualBox - see
>         https://bbs.archlinux.org/viewtopic.php?pid=775414
> k6, k6-2, k6-3, winchip-c6, winchip2 - these are 586-class CPUs
> via c3 c3-2 - see https://bugs.archlinux.org/task/19733 as a proof that
>         Via c3 and c3-Nehemiah don't have nopl
>
> Mikulas
>
> ---
>  lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
> ==================================================================> ---
llvm.orig/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp     2013-11-03
00:24:20.000000000 +0100
> +++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp  2013-11-03
20:57:14.000000000 +0100
> @@ -309,7 +309,10 @@ bool X86AsmBackend::writeNopData(uint64_
>    // This CPU doesnt support long nops. If needed add more.
>    // FIXME: Can we get this from the subtarget somehow?
>    if (CPU == "generic" || CPU == "i386" || CPU ==
"i486" || CPU == "i586" ||
> -      CPU == "pentium" || CPU == "pentium-mmx" || CPU
== "geode") {
> +      CPU == "pentium" || CPU == "pentium-mmx" || CPU
== "i686" ||
> +      CPU == "k6" || CPU == "k6-2" || CPU ==
"k6-3" || CPU == "geode" ||
> +      CPU == "winchip-c6" || CPU == "winchip2" || CPU
== "c3" ||
> +      CPU == "c3-2") {
>      for (uint64_t i = 0; i < Count; ++i)
>        OW->Write8(0x90);
>      return true;
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Tue, 5 Nov 2013, Rafael Espíndola wrote:
> Please include a testcase with the patch.
I'm sending testcase here. Compile it with
"clang -O2 -march=k6-2 -c loop.c"
> gas uses " nopl   0x0(%eax)" for k6_2. Are you sure it is a gas
bug?
Yes, it is gas bug. I should report it to binutils maintainers.

Mikulas
> On 3 November 2013 13:50, Mikulas Patocka
> <mikulas at artax.karlin.mff.cuni.cz> wrote:
> > Hi
> >
> > This patch fixes code generation bug - 586-class CPUs don't
support the
> > nopl instruction and some 686-class CPUs don't support it too.
> >
> > I created bug 17792 for that.
> >
> >
> > BTW. I think you should also optimize padding on these CPUs - instead
of a
> > stream of 0x90 nops, you should generate variants of "lea (%esi),
%esi"
> > instruction like gcc.
> >
> >
> >
> > This patch disables generation of nopl instructions on the following
CPUs:
> >
> > i686 - there are i686-class CPUs that don't have nopl: Via c3,
Transmeta
> >         Crusoe, Microsoft VirtualBox - see
> >         https://bbs.archlinux.org/viewtopic.php?pid=775414
> > k6, k6-2, k6-3, winchip-c6, winchip2 - these are 586-class CPUs
> > via c3 c3-2 - see https://bugs.archlinux.org/task/19733 as a proof
that
> >         Via c3 and c3-Nehemiah don't have nopl
> >
> > Mikulas
> >
> > ---
> >  lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
> > ==================================================================>
> --- llvm.orig/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp     2013-11-03
00:24:20.000000000 +0100
> > +++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp  2013-11-03
20:57:14.000000000 +0100
> > @@ -309,7 +309,10 @@ bool X86AsmBackend::writeNopData(uint64_
> >    // This CPU doesnt support long nops. If needed add more.
> >    // FIXME: Can we get this from the subtarget somehow?
> >    if (CPU == "generic" || CPU == "i386" || CPU ==
"i486" || CPU == "i586" ||
> > -      CPU == "pentium" || CPU == "pentium-mmx" ||
CPU == "geode") {
> > +      CPU == "pentium" || CPU == "pentium-mmx" ||
CPU == "i686" ||
> > +      CPU == "k6" || CPU == "k6-2" || CPU ==
"k6-3" || CPU == "geode" ||
> > +      CPU == "winchip-c6" || CPU == "winchip2" ||
CPU == "c3" ||
> > +      CPU == "c3-2") {
> >      for (uint64_t i = 0; i < Count; ++i)
> >        OW->Write8(0x90);
> >      return true;
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>