This is an archive of the discontinued LLVM Phabricator instance.

[Power9] Added support for the modsw, moduw, modsd, modud hardware instructions that are new to P9.
ClosedPublic

Authored by stefanp on Jun 6 2017, 6:59 AM.

Details

Summary

Note that if we need the result of both the divide and the modulo then we compute the modulo based on the result of the divide and not using the new hardware instruction.

Diff Detail

Repository
rL LLVM

Event Timeline

stefanp created this revision.Jun 6 2017, 6:59 AM
stefanp updated this revision to Diff 101755.Jun 7 2017, 9:16 AM

I did not set the
let Predicates = [IsISA3_0] in {
correctly.
Fixed in this update.

jtony added inline comments.Jun 8 2017, 11:08 AM
lib/Target/PowerPC/PPCISelLowering.cpp
207 ↗(On Diff #101755)

SREM/UREM are actually not instructions, they are SDNode or operations, I think.

test/CodeGen/PowerPC/ppc64-P9-mod.ll
1 ↗(On Diff #101755)

Do we need to test Big-endian also?

nemanjai accepted this revision.Jun 8 2017, 10:07 PM

Aside from a few minor inline nits, LGTM. Please address the inline comments on the commit.

lib/Target/PowerPC/PPCISelLowering.cpp
207 ↗(On Diff #101755)

Sure. Technically. But I think the comment is fine.

8405 ↗(On Diff #101755)

Well, this isn't completely accurate. Probably more accurate to say something like
// Check for a DIV with the same operands as this REM.

8406 ↗(On Diff #101755)

The temporary isn't really needed here. The loop can just be
for (auto UI : Op.getOperand(1)->uses())

test/CodeGen/PowerPC/ppc64-P9-mod.ll
1 ↗(On Diff #101755)

Yes, I agree. The codegen (and therefore CHECK directives) don't need to change, but please add a RUN line with triple -mtriple=powerpc64-unknown-linux-gnu.

20 ↗(On Diff #101755)

For the actual MOD instructions you're checking for where the operands are parameters, please add the input registers to the CHECK directives since they're mandated by the ABI. They should look something like
; CHECK: modsw {{[0-9]+}}, 3, 4

I don't think we care to do that for the checks of existing code though (i.e. the dif/mul/sub).

This revision is now accepted and ready to land.Jun 8 2017, 10:07 PM
inouehrs added inline comments.Jun 8 2017, 10:18 PM
lib/Target/PowerPC/PPCISelLowering.cpp
208 ↗(On Diff #101755)

nit: harware -> hardware

test/CodeGen/PowerPC/ppc64-P9-mod.ll
1 ↗(On Diff #101755)

Also, it is better to add -verify-machineinstrs

stefanp updated this revision to Diff 102196.Jun 12 2017, 9:52 AM
stefanp marked 8 inline comments as done.

Fixed various comments from reviewers.

This revision was automatically updated to reflect the committed changes.