This is an archive of the discontinued LLVM Phabricator instance.

[3/11][POC][Clang][RISCV] Add typedef of the tuple type and define tuple type variant of vlseg2e32
ClosedPublic

Authored by eopXD on Apr 6 2023, 11:53 AM.

Details

Summary

For the cover letter of this patch-set, please checkout D146872.

Depends on D146873.

This is the 3rd patch of the patch-set. This patch originates from
D99593.

Note: This patch is a proof-of-concept and will be extended to full
coverage in the future. Currently, the old non-tuple unit-stride
segment load is not removed, and only signed integer unit-strided
segment load of NF=2, EEW=32 is defined here.

When replacing the old intrinsics, the extra IsTuple parameter under
various places will be redundant and removed.

Authored-by: eop Chen <eop.chen@sifive.com>
Co-Authored-by: Hsiangkai Wang <kai.wang@sifive.com>

Diff Detail

Event Timeline

eopXD created this revision.Apr 6 2023, 11:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2023, 11:53 AM
eopXD requested review of this revision.Apr 6 2023, 11:53 AM
eopXD updated this revision to Diff 511635.EditedApr 7 2023, 12:58 AM

Change: Add overloaded name vlseg2e32_tuple for the tuple type variant
to avoid naming collision to the existing non-tuple type overloaded
intrinsics of vlseg2e32.

This should resolve CI failure of clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/overloaded/vlseg2e32.c.

The overloaded name is temporary as the tuple type variant is targeted to replace the current non-tuple type segment load/store.

eopXD retitled this revision from [3/N][POC][Clang] Add typedef of the tuple type and define tuple type variant of vlseg2e32 to [3/11][POC][Clang] Add typedef of the tuple type and define tuple type variant of vlseg2e32.Apr 10 2023, 12:48 AM
eopXD edited the summary of this revision. (Show Details)
eopXD edited the summary of this revision. (Show Details)
eopXD retitled this revision from [3/11][POC][Clang] Add typedef of the tuple type and define tuple type variant of vlseg2e32 to [3/11][POC][Clang][RISCV] Add typedef of the tuple type and define tuple type variant of vlseg2e32.
craig.topper added inline comments.Apr 18 2023, 4:37 PM
clang/include/clang/AST/ASTContext.h
1486

Use NF or NumFields instead of Tuple?

eopXD updated this revision to Diff 516294.Apr 24 2023, 12:46 AM
eopXD marked an inline comment as done.

Address comments from Craig.

clang/include/clang/AST/ASTContext.h
1486

Changed to NumFields.

craig.topper added inline comments.Apr 26 2023, 10:30 AM
clang/include/clang/Basic/riscv_vector.td
1758

Use an Offset variable like I suggested on the later patches.

eopXD updated this revision to Diff 522923.May 17 2023, 12:11 AM

Address comments from Craig.

eopXD updated this revision to Diff 522924.May 17 2023, 12:17 AM

Update code.

eopXD marked an inline comment as done.May 17 2023, 12:18 AM
evandro removed a subscriber: evandro.May 17 2023, 3:55 PM
craig.topper added inline comments.May 18 2023, 10:39 AM
clang/include/clang/Basic/riscv_vector.td
1757

I don't think we need the Value * variables here. They're only benefit is naming, but I think we can do that with comments.

Something like this

unsigned Offset = IsMasked ? 1 : 0;
Operands.push_back(Ops[Offset]); // Pointer
if (IsMasked)
  Operands.push_back(Ops[0]); // Mask
Operands.push_back(Ops[Offset + 1]); // VL
clang/lib/AST/ASTContext.cpp
4156

Would it be possible to add NumFields to getScalableVectorType maybe with a default value of 1 instead of introducing a new function?

eopXD updated this revision to Diff 523457.May 18 2023, 11:12 AM
eopXD marked 2 inline comments as done.

Address comments from Craig.

craig.topper added inline comments.May 18 2023, 12:40 PM
clang/include/clang/Basic/riscv_vector.td
1760

MaskOperand can still be simplified.

eopXD updated this revision to Diff 523617.May 18 2023, 5:39 PM

Removed another variable that is too verbose.

eopXD marked an inline comment as done.May 18 2023, 5:39 PM
This revision is now accepted and ready to land.May 18 2023, 6:23 PM
eopXD updated this revision to Diff 524182.May 22 2023, 12:54 AM

Bump CI.

eopXD updated this revision to Diff 524187.May 22 2023, 1:19 AM

Bump CI again, the previous patch application failed.

uabelho added inline comments.
clang/include/clang/Support/RISCVVIntrinsicUtils.h
384
/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include/clang/Support/RISCVVIntrinsicUtils.h:384:8: error: private field 'IsTuple' is not used [-Werror,-Wunused-private-field]
  bool IsTuple = false;
       ^
1 error generated.

Seen e.g. here:
https://lab.llvm.org/buildbot/#/builders/57/builds/26989/steps/5/logs/stdio

eopXD marked an inline comment as done.May 22 2023, 3:41 AM
eopXD added inline comments.
clang/include/clang/Support/RISCVVIntrinsicUtils.h
384

Thank you for the report. Resolved this in commit 38140255113297fe3e7926d311cd45004928779e.

eopXD marked an inline comment as done.May 26 2023, 12:23 AM