This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Use Register/MCRegister
ClosedPublic

Authored by mtrofin on Nov 3 2020, 3:22 PM.

Diff Detail

Event Timeline

mtrofin created this revision.Nov 3 2020, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 3 2020, 3:22 PM
mtrofin requested review of this revision.Nov 3 2020, 3:22 PM
google-llvm-upstream-contributions added inline comments.
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
40–41

@critson - Looking at the uses of CondReg and ExecReg, it seems they are always physical registers (never virtual/stack slots), is this correct?

Thanks!

mtrofin added inline comments.Nov 3 2020, 3:25 PM
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
40–41

(sorry, somehow used the wrong account for this comment)

gjain accepted this revision.Nov 3 2020, 3:54 PM
gjain added inline comments.
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
142

Does it make sense that instead of converting a MCRegister to a Register we should just compare the id's? I would think we we'd want to avoid easy conversions of MCRegister to Register.

Another options is we actually provide a comparison operator between MCRegister and Register.

This revision is now accepted and ready to land.Nov 3 2020, 3:54 PM
mtrofin added inline comments.Nov 4 2020, 9:08 AM
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
142

Probably a pair == & != operators would be reasonable; I hit this a couple of times... hmm... may be best to do it in one swoop.

I'd add them in a subsequent patch, wdyt?

gjain added inline comments.Nov 4 2020, 10:06 AM
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp
142

Of course!

This revision was landed with ongoing or failed builds.Nov 4 2020, 12:20 PM
This revision was automatically updated to reflect the committed changes.