Enrich Xmm register with Length Width and datatype information
Needs ReviewPublic

Authored by Abhiroop on Jul 2 2018, 5:20 PM.

Details

Summary

This diff adds length width and the data type information for data residing in an XMM register, so that the cmmRegType and globalRegType function can emit the correct CmmType by looking at an STG GlobalReg

Abhiroop created this revision.Jul 2 2018, 5:20 PM

@carter @AndreasK

I tried adding the Length, Width and data type information without keeping it a Maybe but it was proving to be almost impossible. That patch is here: https://gist.github.com/Abhiroop/ba9cb911385e64484f6ce317ca36aa90 I also tried by adding Length, Width etc info in the globalRegType and cmmRegType function but I hit a dead end with that approach as well. This looks like the most sane way forward.

Abhiroop updated this revision to Diff 17155.Jul 2 2018, 5:34 PM
  • Fix linter issues
Abhiroop updated this revision to Diff 17156.Jul 2 2018, 5:38 PM
  • Fix parsing issue
AndreasK added inline comments.Jul 6 2018, 8:22 AM
compiler/cmm/CmmExpr.hs
426

I think this is the right idea. But can we really have

  • XmmReg 0 (Just ..) (Nothing)
  • XmmReg 0 Nothing (Just ..)

here? It seems either none or both should be present.

Also what does a value of Nothing tell us?
Does it mean we can treat it as whatever, that it's used as scalar register or something else?

If it's used as a vector register isn't width always registerWidth/Length?
In that case storing that just opens you up to this invariant getting violated.

I would suggest you at least inline length and Width into the Register Constructor (XMM, YMM, ...) and specify some invariants.
For example:

  • Length = 1 -> It's a scalar value, look at width the get the type
  • Length = 2+ -> It's a vector of length values each width wide.

But maybe you can come up with something better.

carter added a comment.Jul 6 2018, 3:21 PM

Abi: This seems to be because we currently conflate the "type/represenation" with the physical registers at the CMM level ?

aka ""heres the physical registers" vs "heres the kinda data we want in this class of physical register"

@carter @AndreasK

I tried adding the Length, Width and data type information without keeping it a Maybe but it was proving to be almost impossible. That patch is here: https://gist.github.com/Abhiroop/ba9cb911385e64484f6ce317ca36aa90 I also tried by adding Length, Width etc info in the globalRegType and cmmRegType function but I hit a dead end with that approach as well. This looks like the most sane way forward.

I tried adding the Length, Width and data type information without keeping it a Maybe but it was proving to be almost impossible.

Even so, is there a reason why we would want/need two maybes instead of one?

but I hit a dead end with that approach as well.

But what was the dead end? It's not clear from the patch alone and seems quite tedious to figure out by reverse engineering ;-)