This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Add G_FMAD instruction
ClosedPublic

Authored by arsenm on Sep 6 2019, 7:17 AM.

Diff Detail

Event Timeline

arsenm created this revision.Sep 6 2019, 7:17 AM
aditya_nandakumar accepted this revision.Sep 6 2019, 7:32 AM

Looks good. Is this something that's only produced during legalization of targets?

This revision is now accepted and ready to land.Sep 6 2019, 7:32 AM
arsenm added a comment.Sep 6 2019, 7:42 AM

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

If this is used only by one target (AMDGPU) (and does not require further legalizations), one alternative could be making this a target pseudo.

arsenm added a comment.Sep 6 2019, 8:17 AM

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

If this is used only by one target (AMDGPU) (and does not require further legalizations), one alternative could be making this a target pseudo.

It's already a generic node for the DAG. ARM could use it, but doesn't. In practice it's a huge pain to make this target specific, since it shares most combines with FMA.

We also do have a problem now where we don't really have an equivalent of target specific nodes in GlobalISel. Intrinsics are close, but don't quite fill the purpose and would litter the IR intrinsic namespace

Looks good. Is this something that's only produced during legalization of targets?

It's produced by optimization combines in the DAG mostly, but AMDGPU does use this in a couple custom legalizations

If this is used only by one target (AMDGPU) (and does not require further legalizations), one alternative could be making this a target pseudo.

It's already a generic node for the DAG. ARM could use it, but doesn't. In practice it's a huge pain to make this target specific, since it shares most combines with FMA.

Makes sense.

We also do have a problem now where we don't really have an equivalent of target specific nodes in GlobalISel. Intrinsics are close, but don't quite fill the purpose and would litter the IR intrinsic namespace

The other alternative is to use Target Pseudo instructions with generic operands and it's selectable, but comes with it's drawbacks of not being able to further legalize.

arsenm closed this revision.Sep 6 2019, 1:47 PM

r371254