Page MenuHomePhabricator

[Clang][RISCV] Restrict rvv builtins with zve macros
ClosedPublic

Authored by eopXD on Nov 1 2021, 11:56 PM.

Details

Summary

The zve extension specifies the maximum ELEN for both integer and floating
point mode - defined by macro __riscv_v_elen and __riscv_v_elen_fp.
This commit restricts the functions in riscv_vector.h by the zve defined
macro-s.

Change enum RISCVExtension to RISCVPredefinedMacro since now it
contains not only extensions. Also added type alignment to it.

Diff Detail

Event Timeline

eopXD created this revision.Nov 1 2021, 11:56 PM
eopXD requested review of this revision.Nov 1 2021, 11:56 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 1 2021, 11:56 PM
eopXD edited the summary of this revision. (Show Details)Nov 1 2021, 11:57 PM
eopXD updated this revision to Diff 383985.Nov 1 2021, 11:58 PM

Rebase.

eopXD retitled this revision from [WIP][RISCV] Restrict zvamo, zvlsseg with zve macro-s to [RISCV] Restrict zvamo, zvlsseg with zve macro-s.Nov 2 2021, 12:09 AM
eopXD edited the summary of this revision. (Show Details)
eopXD abandoned this revision.Wed, Jan 19, 11:44 PM

This revision is no longer needed as zvamo and zvlsseg will be removed from arch string.

eopXD reclaimed this revision.Thu, Jan 20, 4:09 AM

This was closed by my misunderstanding.
For clang we still need to do some restrictions based on ELEN.
Reopening.

eopXD retitled this revision from [RISCV] Restrict zvamo, zvlsseg with zve macro-s to [Clang][RISCV] Restrict zvamo, zvlsseg with zve macro-s.Thu, Jan 20, 4:10 AM
eopXD edited the summary of this revision. (Show Details)
eopXD retitled this revision from [Clang][RISCV] Restrict zvamo, zvlsseg with zve macro-s to [Clang][RISCV] Restrict rvv builtin-s with zve macro-s.
eopXD updated this revision to Diff 401684.Thu, Jan 20, 10:16 AM

Update code.

eopXD edited the summary of this revision. (Show Details)Thu, Jan 20, 10:24 AM
eopXD updated this revision to Diff 401687.Thu, Jan 20, 10:28 AM

Update code.

craig.topper added inline comments.Thu, Jan 20, 12:46 PM
clang/utils/TableGen/RISCVVEmitter.cpp
145

Do we need VectorMaxELen32 isn't that the minimum?

799–800

Can D and F go away?

eopXD updated this revision to Diff 401862.Thu, Jan 20, 10:19 PM
eopXD marked 2 inline comments as done.

Rebase and addres comments.
Resolve conflcits due to zvlsseg removal.

clang/utils/TableGen/RISCVVEmitter.cpp
145

Yes you are correct. We don't need it.

799–800

Done.

eopXD updated this revision to Diff 401870.Thu, Jan 20, 11:16 PM

Follow clang-format.

craig.topper added inline comments.Fri, Jan 21, 10:22 AM
clang/utils/TableGen/RISCVVEmitter.cpp
143–144

Drop F and D here?

1316–1317

Drop F and D here

eopXD updated this revision to Diff 402036.Fri, Jan 21, 10:26 AM
eopXD marked an inline comment as done.

Cleanup unused enum.

This revision is now accepted and ready to land.Fri, Jan 21, 10:28 AM
eopXD edited the summary of this revision. (Show Details)Fri, Jan 21, 10:31 AM
eopXD edited the summary of this revision. (Show Details)

LGTM too. Though the commit title and message has hyphens in places I wouldn't expect them. macros and builtins is fine.

clang/utils/TableGen/RISCVVEmitter.cpp
797

nit: This still says "extensions"

eopXD retitled this revision from [Clang][RISCV] Restrict rvv builtin-s with zve macro-s to [Clang][RISCV] Restrict rvv builtins with zve macros.Fri, Jan 21, 10:34 AM

LGTM too. Though the commit title and message has hyphens in places I wouldn't expect them. macros and builtins is fine.

Thank you to also drop by and take a look :)

clang/utils/TableGen/RISCVVEmitter.cpp
797

Yeah I thought of not changing the variable name, however as other enums are added in keeping it as "extensions" is a bit strange to me.

This revision was landed with ongoing or failed builds.Fri, Jan 21, 10:39 AM
This revision was automatically updated to reflect the committed changes.