This is an archive of the discontinued LLVM Phabricator instance.

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

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

Details

Summary

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

Diff Detail

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
7306

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
7308

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.Sep 12 2021, 7:58 PM

Addressed review comments.

saghir updated this revision to Diff 372505.Sep 14 2021, 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.Sep 14 2021, 9:50 AM
saghir edited the summary of this revision. (Show Details)
saghir updated this revision to Diff 374529.Sep 23 2021, 6:18 AM

Changed approach for Sema checks.

saghir updated this revision to Diff 375629.Sep 28 2021, 10:03 AM

Stripped RV qualifiers for Sema checking.

nemanjai accepted this revision.Sep 28 2021, 4:49 PM

LGTM.

This revision is now accepted and ready to land.Sep 28 2021, 4:49 PM
This revision was landed with ongoing or failed builds.Oct 12 2021, 6:51 AM
This revision was automatically updated to reflect the committed changes.