While looking through various .td files, I came across a beautiful recursive (!) class that builds a list of N copies of the same record. This must have been written before !listsplat existed. It now uses !listsplat.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Thank you for cleaning this up.
If you're tinkering with this, another possible cleanup would be to use !setdagop instead of !foreach(...,!subst())
E.g. https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/NVPTX/NVPTXIntrinsics.td#L7497
PatFrag IntrFrag = PatFrag<PFOperands,
!foreach(tmp, PFOperands, !subst(ops, Intr, tmp)), #### <==== this
!cond(!eq(Space, ".shared"): AS_match.shared,
!eq(Space, ".global"): AS_match.global,
1: AS_match.generic)>;Comment Actions
You're quite welcome. I will work on your additional simplification and see if I can find other similar bits of code.
Comment Actions
@tra There are other things I can clean up in NVPTXIntrinsics.td. Can I do this without us spending the rest of our lives sorting out conflicts?
Comment Actions
Go for it. E.g. texture/surface intrinsics could be rewritten more concisely.
I'm not sure what kind of conflicts do you have in mind. Could you give me an example?
Comment Actions
Let's see . . .
First, it's clear that we need a more powerful version of !subst. I'm starting to think about !replace. But that's a separate task. I've also got some ideas for bang operators to manipulate dags.
It looks like there are some !foreach that can be !listsplat:
// Instruction input/output arguments for the fragment. list<NVPTXRegClass> ptx_regs = !foreach(tmp, regs, regclass);
The code you mentioned above:
dag PFOperands = !if(WithStride, (ops node:$src, node:$ldm), (ops node:$src));
// Build PatFrag that only matches particular address space.
PatFrag IntrFrag = PatFrag<PFOperands,
!foreach(tmp, PFOperands, !subst(ops, Intr, tmp)),It's more efficient simply to assign the substituted dag first:
dag PFOperandsIntr = !if(WithStride, (Intr node:$src, node:$ldm), (Intr node:$src));
We can also use the new !sub operator to make things clearer.
These NVPTX TableGen files are quite clean already.
Comment Actions
It looks like there are some !foreach that can be !listsplat:
// Instruction input/output arguments for the fragment. list<NVPTXRegClass> ptx_regs = !foreach(tmp, regs, regclass);
Yup.
The code you mentioned above:
dag PFOperands = !if(WithStride, (ops node:$src, node:$ldm), (ops node:$src)); // Build PatFrag that only matches particular address space. PatFrag IntrFrag = PatFrag<PFOperands, !foreach(tmp, PFOperands, !subst(ops, Intr, tmp)),It's more efficient simply to assign the substituted dag first:
dag PFOperandsIntr = !if(WithStride, (Intr node:$src, node:$ldm), (Intr node:$src));
Good catch. That would work in this case.
Still, I think there are more !foreach instances where you'd still need to use !setdagop as we're not always constructing the dag from scratch.
DAG manipulation has been one of the trickier things to do in tablegen at the time I was messing with NVPTX tablegen files.
@nhaehnle has improved tablegen quite a bit since then.
We can also use the new !sub operator to make things clearer.
SGTM.
Send me a patch and I'll take a look.
Comment Actions
Okay, I'll open a new revision now. I worked on NVPTXIntrinsics.td. If you like where that is going, I'll do similar work on the remaining .td files.