Page MenuHomePhabricator

[PowerPC] Allow MMA built-ins to accept restrict and volatile qualified pointers
Needs ReviewPublic

Authored by saghir on Jul 22 2021, 7:49 AM.

Details

Reviewers
nemanjai
stefanp
Group Reviewers
Restricted Project
Summary

This patch allows MMA built-ins on PowerPC to accept restrict
and volatile qualified pointers.

Diff Detail

Unit TestsFailed

TimeTest
390 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-cxa-atexit.S
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-cxa-atexit.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-cxa-atexit.S
350 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-static-initializer.S
Script: -- : 'RUN: at line 7'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-static-initializer.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-static-initializer.S

Event Timeline

saghir created this revision.Jul 22 2021, 7:49 AM
saghir requested review of this revision.Jul 22 2021, 7:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 22 2021, 7:49 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
saghir added reviewers: Restricted Project, nemanjai, stefanp.Jul 22 2021, 7:51 AM
nemanjai requested changes to this revision.Jul 30 2021, 6:13 AM
nemanjai added inline comments.
clang/lib/Sema/SemaChecking.cpp
7444–7447

Parentheses here seem superfluous. Also, am I reading this correctly? Does this mean that if the argument type is __restrict anything, we are accepting it? That is not the intent here. We don't want to accept just ANY restrict qualified type. We want to accept either: ExpectedType or ExpectedType __restrict.

This revision now requires changes to proceed.Jul 30 2021, 6:13 AM
saghir retitled this revision from [PowerPC] Allow MMA builtins to accpet restrict qualified pointers to [PowerPC] Allow MMA builtins to accept restrict qualified pointers.Aug 5 2021, 7:30 AM
saghir updated this revision to Diff 364467.Aug 5 2021, 7:50 AM

Addressed review comments. Added another test case.

saghir retitled this revision from [PowerPC] Allow MMA builtins to accept restrict qualified pointers to [PowerPC] Allow MMA built-ins to accept restrict qualified pointers.Aug 6 2021, 7:50 AM
saghir edited the summary of this revision. (Show Details)
saghir added projects: Restricted Project, Restricted Project.
nemanjai requested changes to this revision.Aug 10 2021, 4:14 PM
nemanjai added inline comments.
clang/lib/Sema/SemaChecking.cpp
7446

I am not in favour of the "single use lambda" idiom. We don't really need a lambda here. Also, this will handle restrict, but it won't handle const/volatile which also shouldn't be a problem for the loads.

I think we should do the following:

  • Change the signatures in the .def file to specify all allowed const/volatile/restrict qualifiers
  • Add the qualifiers in DecodePPCMMATypeFromStr()
  • Ensure that the expected and arg types match with hasSameUnqualifiedType()
  • Ensure that the actual type is not more qualified than the expected type (for example, if the expected type doesn't have const/restrict/volatile and the actual type does, we have a type mismatch).
This revision now requires changes to proceed.Aug 10 2021, 4:14 PM
saghir updated this revision to Diff 372151.Sun, Sep 12, 7:58 PM

Addressed review comments.

saghir updated this revision to Diff 372505.Tue, Sep 14, 9:36 AM

update test case name to be consistent with existing ones.

saghir retitled this revision from [PowerPC] Allow MMA built-ins to accept restrict qualified pointers to [PowerPC] Allow MMA built-ins to accept restrict and volatile qualified pointers.Tue, Sep 14, 9:50 AM
saghir edited the summary of this revision. (Show Details)
saghir updated this revision to Diff 374529.Thu, Sep 23, 6:18 AM

Changed approach for Sema checks.