Page MenuHomePhabricator

[mlir][openacc] Switch to assembly format for acc.data
ClosedPublic

Authored by clementval on Sep 25 2020, 12:14 PM.

Details

Summary

This patch remove the printer/parser for the acc.data operation since its syntax
fits nicely with the assembly format. It reduces the maintenance for this op.

Diff Detail

Unit TestsFailed

TimeTest
26,310 mslinux > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm

Event Timeline

clementval created this revision.Sep 25 2020, 12:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2020, 12:14 PM
clementval requested review of this revision.Sep 25 2020, 12:14 PM

I understand that this patch is just changing to assembly format but #just-asking, the following clauses are missing.

if( condition )
deviceptr( var-list )
default( none | present )

mlir/test/Dialect/OpenACC/ops.mlir
124

Is this separation of variables and types due to using assembly format? The previous style might be better for longer heterogeneous lists.

I understand that this patch is just changing to assembly format but #just-asking, the following clauses are missing.

if( condition )
deviceptr( var-list )
default( none | present )

This is part of a patch series. Look at D88331.

clementval added inline comments.Sep 26 2020, 6:11 AM
mlir/test/Dialect/OpenACC/ops.mlir
124

Yeah this is because of the assembly format. I'm not aware of a way to keep the previous style without custom parser. All operation I have seen using assembly format have variadic operand set this way.

Thanks. LGTM.

This revision is now accepted and ready to land.Sep 26 2020, 6:23 AM

Thanks. LGTM.

Thanks for the review. If you have time can you have a look at D88326 since it is the first patch of the serie.

This revision was landed with ongoing or failed builds.Sep 27 2020, 6:21 PM
This revision was automatically updated to reflect the committed changes.