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

Nikolai Bozhenov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 11 09:35:59 PST 2017


n.bozhenov added a comment.

In https://reviews.llvm.org/D28196#642763, @spatel wrote:

> In https://reviews.llvm.org/D28196#642439, @n.bozhenov wrote:
>
> > Rebased atom-bypass-slow-division*.ll tests on https://reviews.llvm.org/D28469.
> >
> > Didn't rebase slow-div.ll because in the next patch I would add several RUN lines into the test which produce similar but not exactly the same assembly. So, I decided to check here only whether the transformation is applied or not.
>
>
> Please correct me if I'm wrong, but this patch is now making 2 functional changes, but only testing for the 1st one:
>
> 1. Fix the definition of FeatureSlowDivide64 for existing CPUs (Bonnell/Silvermont)
> 2. Add the FeatureSlowDivide64 to SNB and later big cores
>
>   If that's correct, you should remove the 2nd code change from this patch and submit it as a follow-up to this patch with a corresponding test. Alternatively, you could bring the minimum test changes from the other test file back into this patch since that's a small difference. But we don't want to have a functional change with no testing. I know this is a patch series with appropriate testing at the end, but it's important to have the right tests in at every step just in case one or more of those steps needs to be reverted.


Does it look better now? I have added a Sandy Bridge test into atom-bypass-slow-division-64.ll


https://reviews.llvm.org/D28196





More information about the llvm-commits mailing list