[PATCH] D50222: [CodeGen] [SelectionDAG] More efficient code for X % C == 0 (UREM case)

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 11 08:38:24 PDT 2018


lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Updated the tests in https://reviews.llvm.org/rL341953.
As far i'm concerned this is good to go.
Vector and vector non-splat support sounds like a great task for the first follow-up :)
But, still let's wait for others to comment for a bit..



================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3799
+  APInt D0 = D.lshr(K);
+
+  // P = inv(D0, 2^W)
----------------
hermord wrote:
> lebedev.ri wrote:
> > Ok, thank you for the explanation regarding `D0 != 1`.
> > I think here now we can just add an assert, to document it.
> > ```
> > APInt D0 = D.lshr(K);
> > 
> > assert(!D0.isOneValue() && "The fold is invalid for D0 of 1. Unreachable since we leave powers-of-two to be improved elsewhere.");
> > ```
> This turns out to actually be reachable: the logic that handles REM by powers of two is in `visitREM`, which is invoked after `visitSetCC` from where we would've come here. I kept this as an early return if that's OK.
Sounds good.


================
Comment at: test/CodeGen/X86/urem-seteq.ll:4
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s --check-prefixes=CHECK,X64,NOBMI2
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -mattr=+bmi2 < %s | FileCheck %s --check-prefixes=CHECK,X64,BMI2
+
----------------
hermord wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > Actually, looking at the check-lines, i'm not sure we want to check the bmi2 version.
> > > I'm not seeing anything there that would benefit from anything above baseline, i think.
> > I've meant to drop this check-line, but apparently forgot.
> > I'm not 100% sure if we want/don't want it.
> Should I drop it? On a related note, I could add `AVX2` to the vector tests on `X86` if that's likely to be useful.
Right, good idea.
I checked all the various sse/avx versions, and kept the unique ones that make a difference here.


Repository:
  rL LLVM

https://reviews.llvm.org/D50222





More information about the llvm-commits mailing list