Page MenuHomePhabricator

[RISCV] Fix vm operand constraint to fit GCC's behavior
AcceptedPublic

Authored by kito-cheng on Oct 27 2021, 1:21 AM.

Details

Summary
  • vm constraint is used for masking operand, which always v0.
  • Update testcase, only masking operand should use vm, vector mask operations should just use vr for any vector register.
    • Revise the description of vm constraint.
  • This patch also fix issue on RISCVRegisterInfo.td and RISCVISelLowering.cpp.

    RISCVRegisterInfo.td:
    • The first VT in the list must be the largest total size since the SelectionDAGBuilder uses the first register in the list as the canonical type for the register.

      RISCVISelLowering.cpp:
    • Fix RISCVTargetLowering::splitValueIntoRegisterParts and RISCVTargetLowering::joinRegisterPartsIntoValue for handling vectors with different total size, that will happened on fractional LMUL since fractional LMUL is always occupy one vector register.

Diff Detail

Unit TestsFailed

TimeTest
100 msx64 debian > Polly.CodeGen::aliasing_different_pointer_types.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen -polly-codegen-verify -polly-codegen -S < /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_different_pointer_types.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_different_pointer_types.ll
80 msx64 debian > Polly.CodeGen::aliasing_parametric_simple_1.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen -polly-codegen-verify -polly-codegen -S < /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_parametric_simple_1.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_parametric_simple_1.ll
80 msx64 debian > Polly.CodeGen::aliasing_parametric_simple_2.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen -polly-codegen-verify -polly-codegen -S < /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_parametric_simple_2.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/aliasing_parametric_simple_2.ll
160 msx64 debian > Polly.CodeGen::exprModDiv.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen -polly-codegen-verify -polly-import-jscop -polly-codegen -S < /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/exprModDiv.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/exprModDiv.ll
180 msx64 debian > Polly.CodeGen::invariant_load_base_pointer_conditional_2.ll
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen -polly-codegen-verify -analyze -polly-scops -polly-invariant-load-hoisting=true < /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/invariant_load_base_pointer_conditional_2.ll | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/polly/test/CodeGen/invariant_load_base_pointer_conditional_2.ll
View Full Test Results (21 Failed)

Event Timeline

kito-cheng created this revision.Oct 27 2021, 1:21 AM
kito-cheng requested review of this revision.Oct 27 2021, 1:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2021, 1:21 AM
kito-cheng retitled this revision from Fix vm operand constraint to fit GCC's behavior to [RISCV] Fix vm operand constraint to fit GCC's behavior.Oct 27 2021, 1:23 AM
frasercrmck added inline comments.Oct 27 2021, 3:32 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9861

The name LargerEltTypeVT, to me anyway, implies it's a vector type with a larger element type when in fact it's a vector with the same element type. Maybe something like WideValueVT?

EDIT: Though that said, in joinRegisterPartsIntoValue you're keeping the name SameEltTypeVT. I'd prefer consistency in whatever we pick.

9865

I realise you're copying this from the existing code but shouldn't we be using DAG.getVectorIdxConstant(0, DL) here?

9868

I'm wondering if we should place braces around this, given the else is a multi-line statement. I think we do that elsewhere.

llvm/lib/Target/RISCV/RISCVRegisterInfo.td
529–530

Good catch! I'd prefer to properly order them as in VM below though. Which begs the question: why isn't VM using VMaskVTs?

kito-cheng added inline comments.Oct 27 2021, 9:24 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
9865

I'll update what I updated part, and keep untouched for other DAG.getConstant(0, DL, Subtarget.getXLenVT())

kito-cheng marked 4 inline comments as done.
frasercrmck accepted this revision.Thu, Oct 28, 2:34 AM

LGTM but I'd prefer to wait a couple of days to give other people a chance to look

This revision is now accepted and ready to land.Thu, Oct 28, 2:34 AM