[PATCH] D28196: [X86] Tune bypassing of slow division for Intel CPUs

[Nikolai Bozhenov via Phabricator via llvm-commits]

n.bozhenov added inline comments.


================
Comment at: lib/Target/X86/X86.td:214
                                      "HasSlowDivide64", "true",
-                                     "Use 16-bit divide for positive values less than 65536">;
+                                     "Use 32-bit divide for positive values less than 2^32">;
 def FeaturePadShortFunctions : SubtargetFeature<"pad-short-functions",
----------------
RKSimon wrote:
> Is losing the idivq-to-divw option likely to cause any problems to existing users?
> Is losing the idivq-to-divw option likely to cause any problems to existing users?
Well, in theory it is possible for Atoms, because DIV16 is somewhat faster for them than DIV32. But still DIV64 is way way slower than DIV32, so this change is probably a win even for Atoms because obviously DIV32 can be taken more often than DIV16. For all other Intel architectures (including Silvermont) there's no sense in idivq-to-divw transformation.


================
Comment at: test/CodeGen/X86/atom-bypass-slow-division-64.ll:3
+; RUN: llc < %s -mcpu=silvermont -march=x86-64 -asm-verbose=false | FileCheck %s
+; RUN: llc < %s -mcpu=skylake    -march=x86-64 -asm-verbose=false | FileCheck %s
 
----------------
RKSimon wrote:
> spatel wrote:
> > Is there some reason to choose skylake here rather than sandybridge? If not, I'd prefer SNB because that's the oldest big core where the feature is applied?
> Drop the -march and move the triple in there instead.
> Is there some reason to choose skylake here rather than sandybridge?
It doesn't make much difference because in D28197 I change this into `-mattr=+idivq-to-divl`


================
Comment at: test/CodeGen/X86/atom-bypass-slow-division-64.ll:3
+; RUN: llc < %s -mcpu=silvermont -march=x86-64 -asm-verbose=false | FileCheck %s
+; RUN: llc < %s -mcpu=skylake    -march=x86-64 -asm-verbose=false | FileCheck %s
 
----------------
n.bozhenov wrote:
> RKSimon wrote:
> > spatel wrote:
> > > Is there some reason to choose skylake here rather than sandybridge? If not, I'd prefer SNB because that's the oldest big core where the feature is applied?
> > Drop the -march and move the triple in there instead.
> > Is there some reason to choose skylake here rather than sandybridge?
> It doesn't make much difference because in D28197 I change this into `-mattr=+idivq-to-divl`
> Drop the -march and move the triple in there instead.
Ok, I will. But what is the difference?


https://reviews.llvm.org/D28196