Page MenuHomePhabricator

More extendable LaneBitmask
Needs ReviewPublic

Authored by kparzysz on Jul 20 2017, 9:29 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

This is in response to recent emails about extending the numbers of available lanes. While that problem went away, this is a more generic approach than what we have at the moment.
I haven't tested it beyond "check-llvm". I'm wondering what you think.

Diff Detail

Repository
rL LLVM

Event Timeline

kparzysz created this revision.Jul 20 2017, 9:29 AM
kparzysz added inline comments.Jul 20 2017, 9:31 AM
lib/CodeGen/MIRParser/MIParser.cpp
548

This part is not complete.

MatzeB edited edge metadata.Jul 20 2017, 10:49 AM
  • I would be slightly hesitant here, as it complicates code and none of the public targets needs this. We don't even try the "uint64_t" instead of "unsigned" step but instead just make it all complicated right away...
  • I would not commit this without careful measurements to ensure that compilers indeed optimize all of this away and we don't have a compiletime impact.
  • You probably can use some of the APInt::tcXXX functions to avoid code duplication.

Yeah, there is no need for it right now. I was just curious about this kind of approach.

I can keep this as a patch here in case this issue comes back later.

craig.topper edited edge metadata.Jul 20 2017, 11:26 AM

Can we go ahead and add getHighestLane to the existing class to hide the Log2_32? Maybe getNumLanes as well.

Done in r308655 and r308658.

MatzeB resigned from this revision.Jan 11 2018, 10:12 AM
craig.topper resigned from this revision.Jan 12 2018, 9:42 AM
kparzysz updated this revision to Diff 142659.Apr 16 2018, 10:06 AM
kparzysz removed reviewers: craig.topper, MatzeB.
hahmed added a subscriber: hahmed.Apr 16 2018, 10:51 PM

I m working on huge hardware and I need this patch to work. When I tested this for N=128 I am getting error like "Must be a leaf subregister".

craig.topper added inline comments.
include/llvm/MC/LaneBitmask.h
149

I think this leaves all the other entries in T as garbage. This could explain the error hameeza is getting.

Try changing this to

Type T = {};
kparzysz added inline comments.Apr 17 2018, 11:10 AM
include/llvm/MC/LaneBitmask.h
149

Good catch.

Btw, feel free to reinstate yourself as a reviewer if you feel so inclined...