Page MenuHomePhabricator

[RISCV] Disable EEW=64 for index values when XLEN=32.
Needs ReviewPublic

Authored by jacquesguan on Jul 21 2021, 8:44 PM.

Details

Summary

According to the 1.0-rc1, 18.2 : The V extension supports all vector load and store instructions (Section Vector Loads and Stores), except the V extension does not support EEW=64 for index values when XLEN=32, disable EEW=64 for vector index load/store when XLEN=32.

Diff Detail

Unit TestsFailed

TimeTest
2,580 msx64 debian > libarcher.critical::critical.c
Script: -- : 'RUN: at line 15'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/critical/critical.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/critical/Output/critical.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/critical/Output/critical.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/critical/Output/critical.c.tmp.log | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/critical/critical.c
2,730 msx64 debian > libarcher.parallel::parallel-simple.c
Script: -- : 'RUN: at line 15'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/parallel/parallel-simple.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-simple.c.tmp -latomic && env OMP_TOOL_VERBOSE_INIT=stderr env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-simple.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-simple.c.tmp.log 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/parallel/parallel-simple.c --check-prefixes CHECK,TSAN_ON
2,810 msx64 debian > libarcher.races::critical-unrelated.c
Script: -- : 'RUN: at line 13'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/critical-unrelated.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/critical-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/deflake.bash /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/critical-unrelated.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/critical-unrelated.c.tmp.log | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/critical-unrelated.c
2,960 msx64 debian > libarcher.races::lock-nested-unrelated.c
Script: -- : 'RUN: at line 13'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-nested-unrelated.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-nested-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/deflake.bash /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-nested-unrelated.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-nested-unrelated.c.tmp.log | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-nested-unrelated.c
2,790 msx64 debian > libarcher.races::lock-unrelated.c
Script: -- : 'RUN: at line 13'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests -I /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/runtime/src -L /var/lib/buildkite-agent/builds/llvm-project/build/lib -Wl,-rpath,/var/lib/buildkite-agent/builds/llvm-project/build/lib /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/deflake.bash /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp 2>&1 | tee /var/lib/buildkite-agent/builds/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp.log | /var/lib/buildkite-agent/builds/llvm-project/build/./bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c
View Full Test Results (17 Failed)

Event Timeline

jacquesguan created this revision.Jul 21 2021, 8:44 PM
jacquesguan requested review of this revision.Jul 21 2021, 8:44 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 21 2021, 8:44 PM

Why do they need to be disabled? Doesn’t the spec define them to truncate?

jrtc27 added inline comments.Jul 21 2021, 8:57 PM
clang/include/clang/Basic/riscv_vector.td
555

Ignoring whether the change is actually correct, this should be capitalised as XLen32EEWList, but really this should actually be RV32 not XLen32 as that's not a term we use.

689

Xlen64 is not an extension. Nor is RV64I, even, it is a base ISA, but that would at least be somewhat defensible. In fact, Xlen64 would be parsed as a valid non-standard extension called "Xlen" with major version 64 and minor version 0, just like any other Xfoo.

Why do they need to be disabled? Doesn’t the spec define them to truncate?

In the 1.0-rc1, 18.2: The V extension supports all vector load and store instructions (Section Vector Loads and Stores), except the V extension does not support EEW=64 for index values when XLEN=32.

I think this means that all index instruction with eew=64 is only supported in RV64.

Why do they need to be disabled? Doesn’t the spec define them to truncate?

In the 1.0-rc1, 18.2: The V extension supports all vector load and store instructions (Section Vector Loads and Stores), except the V extension does not support EEW=64 for index values when XLEN=32.

I think this means that all index instruction with eew=64 is only supported in RV64.

Thank you. Can you put that in the patch description so it gets into the commit log.

jacquesguan edited the summary of this revision. (Show Details)Jul 21 2021, 11:40 PM
jacquesguan added inline comments.Jul 21 2021, 11:53 PM
clang/include/clang/Basic/riscv_vector.td
689

So change Xlen64 to RV64 or create a new field of RVVBuiltin to describle it? Which one do you think is better?

frasercrmck added inline comments.Jul 22 2021, 3:30 AM
clang/include/clang/Basic/riscv_vector.td
555

While we're here I'm wondering whether a top-level Xlen32EEWList/RV32EEWList is conveying the wrong thing. It's only the loads and stores that have a different EEW list on RV32, isn't it?

jacquesguan added inline comments.Jul 22 2021, 4:07 AM
clang/include/clang/Basic/riscv_vector.td
555

Yes, only for index load/store, we should add the macro to the generated header to make EEW=64 just available on RV64.

HsiangKai added inline comments.Jul 22 2021, 9:00 AM
clang/include/clang/Basic/riscv_vector.td
676

There is no need to define Xlen32EEWList. You could use EEWList[0-2] for the purpose.

remove Xlen32EEWList and rename Xlen64 to RV64.

clang/include/clang/Basic/riscv_vector.td
676

Done, thank you.

craig.topper added inline comments.Aug 1 2021, 7:55 PM
clang/utils/TableGen/RISCVVEmitter.cpp
179

RequiredExtensions should be a reference

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
349

This would just print a message to stderr but wouldn't fail the program. Probably should use report_fatal_error. Or we could let the caller go to SelectCode which would also trigger a "Cannot select" fatal error.

430

Same as above

Replace errs with report_fatal_error and change the vector argument type to reference type.

jacquesguan added inline comments.Aug 1 2021, 8:37 PM
clang/utils/TableGen/RISCVVEmitter.cpp
179

Done, thanks.

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
349

Done, thanks.

430

Done, thanks.

Truncate i64 index vector for vector gather/scatter in RV32 to avoid using index load instruction that has EEW=64.

remove newlines in error string.