This is an archive of the discontinued LLVM Phabricator instance.

[NVPTX] Fix sign/zero-extending ldg/ldu instruction selection
ClosedPublic

Authored by jholewinski on Apr 27 2016, 1:40 PM.

Details

Summary

We don't have sign-/zero-extending ldg/ldu instructions defined,
so we need to emulate them with explicit CVTs. We were originally
handling the i8 case, but not any other cases.

Fixes PR26185

Diff Detail

Repository
rL LLVM

Event Timeline

jholewinski retitled this revision from to [NVPTX] Fix sign/zero-extending ldg/ldu instruction selection.
jholewinski updated this object.
jholewinski added reviewers: jingyue, jlebar.

Just putting this up for Tobias to try... test case incoming.

Hi Justin,

thank you for this fix. This resolves the problem I have seen earlier.

jingyue edited edge metadata.Apr 28 2016, 9:17 AM

Awesome! Thanks Justin.

lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
2080 ↗(On Diff #55306)

The giant if-switch below may be simplified by

DestType = OrigType.getSimpleVT().SimpleTy;
if (SizeOf(EltVT) < SizeOf(DestType)) {
  CvtOpc = GetConvertOpcode(EltVT, DestType);
  ...
}
jholewinski added inline comments.Apr 28 2016, 9:29 AM
lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
2080 ↗(On Diff #55306)

You're suggesting pulling the logic out into a utility function? Or am I just missing your point?

jingyue added inline comments.Apr 28 2016, 9:59 AM
lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
2080 ↗(On Diff #55306)

Yes, I am suggesting some refactoring.

jholewinski edited edge metadata.

Refactor opcode selection into utility function and add test case

jingyue accepted this revision.Apr 28 2016, 1:17 PM
jingyue edited edge metadata.

Looks great! Thanks.

lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
5191 ↗(On Diff #55470)

You can directly return IsSigned ? ... : ... so you don't need break for each case.

lib/Target/NVPTX/NVPTXISelDAGToDAG.h
96 ↗(On Diff #55470)

Is this method static?

This revision is now accepted and ready to land.Apr 28 2016, 1:17 PM
jholewinski edited edge metadata.

Address review comments

This revision was automatically updated to reflect the committed changes.