Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle two issues that came up after some AVE improvements #26629

Merged
merged 2 commits into from
Jan 31, 2025
Merged

Handle two issues that came up after some AVE improvements #26629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19493a1
Fix and add forgotten good files
e-kayrakli Jan 29, 2025
fd5521c
Workaround an issue by avoiding optimizing const, multid arrays
e-kayrakli Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions compiler/optimizations/arrayViewElision.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@
// `--report-array-view-elision` flag to enable some output during compilation
// to help with understanding what's optimized and what's not.

static const char* createConstProtoSlice = "chpl__createConstProtoSlice";
static const char* createProtoSlice = "chpl__createProtoSlice";

ArrayViewElisionTransformer::ArrayViewElisionTransformer(CallExpr* origCall):
origCall_(origCall) {
Expand Down Expand Up @@ -181,8 +183,7 @@ CallExpr* ArrayViewElisionTransformer::genCreateProtoSlice(CallExpr* call) {

const bool isConst = base->symbol()->isConstant();

const char* factory = isConst ? "chpl__createConstProtoSlice" :
"chpl__createProtoSlice";
const char* factory = isConst ? createConstProtoSlice : createProtoSlice;

CallExpr* ret = new CallExpr(factory, base->copy());
for_actuals(actual, call) {
Expand Down Expand Up @@ -341,6 +342,11 @@ bool ArrayViewElisionPrefolder::handleOneProtoSlice(bool isLhs) {
}

CallExpr* typeCheck = new CallExpr("chpl__ave_exprCanBeProtoSlice");

// add the isConst flag for a workaround
// See: https://github.com/chapel-lang/chapel/issues/26626
typeCheck->insertAtTail(call->isNamed(createConstProtoSlice) ? gTrue:gFalse);

for_actuals (actual, call) {
INT_ASSERT(isSymExpr(actual));
typeCheck->insertAtTail(actual->copy());
Expand Down
19 changes: 12 additions & 7 deletions modules/internal/ChapelArrayViewElision.chpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module ChapelArrayViewElision {
// of these functions.

proc chpl__createProtoSlice(ref Arr, slicingExprs ...)
where chpl__createProtoSliceArgCheck(Arr, slicingExprs) {
where chpl__createProtoSliceArgCheck(Arr, slicingExprs, isConst=false) {

if slicingExprs.size == 1 then
return new chpl__protoSlice(isConst=false, c_addrOf(Arr),
Expand All @@ -43,7 +43,7 @@ module ChapelArrayViewElision {
}

proc chpl__createConstProtoSlice(const ref Arr, slicingExprs ...)
where chpl__createProtoSliceArgCheck(Arr, slicingExprs) {
where chpl__createProtoSliceArgCheck(Arr, slicingExprs, isConst=true) {

if slicingExprs.size == 1 {
return new chpl__protoSlice(isConst=true, c_addrOfConst(Arr),
Expand All @@ -68,8 +68,9 @@ module ChapelArrayViewElision {
return new chpl__protoSlice();
}

proc chpl__ave_exprCanBeProtoSlice(base, idxExprs...) param: bool {
return chpl__ave_baseTypeSupports(base) &&
proc chpl__ave_exprCanBeProtoSlice(param isConst: bool,
base, idxExprs...) param: bool {
return chpl__ave_baseTypeSupports(base, isConst) &&
chpl__ave_idxExprsSupport(base.idxType, (...idxExprs)) &&
!chpl__ave_nonCompliantSlice(base, (...idxExprs));
}
Expand Down Expand Up @@ -358,18 +359,22 @@ module ChapelArrayViewElision {
// private interface
//

private proc chpl__createProtoSliceArgCheck(Arr, slicingExprs) param: bool {
private proc chpl__createProtoSliceArgCheck(Arr, slicingExprs,
param isConst) param: bool {
compilerAssert(isTuple(slicingExprs));

return chpl__ave_baseTypeSupports(Arr) &&
return chpl__ave_baseTypeSupports(Arr, isConst) &&
(chpl__isTupleOfRanges(slicingExprs) ||
(slicingExprs.size == 1 && isDomain(slicingExprs[0])) ||
_validRankChangeArgs(slicingExprs, Arr.idxType));
}

private proc chpl__ave_baseTypeSupports(base) param: bool {
private proc chpl__ave_baseTypeSupports(base,
param isConst: bool) param: bool {
import Reflection;
return isArray(base) && !chpl__isArrayView(base) && // we could allow views
// See https://github.com/chapel-lang/chapel/issues/26626 for below
(!isConst || base.rank == 1) &&
base.domain.supportsArrayViewElision() &&
Reflection.canResolve("c_addrOf", base);
}
Expand Down
46 changes: 46 additions & 0 deletions test/optimizations/arrayViewElision/localToDistributed.comm-none.good
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
ArrayViewElision supported localToDistributed.chpl:11
lhsBaseType: [BlockDom(1,int(64),one,unmanaged DefaultDist)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

ArrayViewElision supported localToDistributed.chpl:24
lhsBaseType: [CyclicDom(1,int(64),one)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

ArrayViewElision supported localToDistributed.chpl:35
lhsBaseType: [StencilDom(1,int(64),one,false)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

Performing protoSlice bulk transfer
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
In DefaultRectangular._simpleTransfer(): Alo=(3,), Blo=(3,), len=3, elemSize=8
operator =(a:[],b:[]): successfully completed bulk transfer
operator =(a:[],b:[]): successfully completed bulk transfer
1 1 2 2 2 1 1 1 1 1
Performing protoSlice bulk transfer
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): bulk transfer did not happen
1 1 2 2 2 1 1 1 1 1
Performing protoSlice bulk transfer
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
In DefaultRectangular._simpleTransfer(): Alo=(3,), Blo=(3,), len=3, elemSize=8
operator =(a:[],b:[]): successfully completed bulk transfer
operator =(a:[],b:[]): successfully completed bulk transfer
1 1 2 2 2 1 1 1 1 1
14 changes: 11 additions & 3 deletions test/optimizations/arrayViewElision/localToDistributed.good
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,35 @@
ArrayViewElision supported localToDistributed.chpl:11
ArrayViewElision supported (dynamic locality check required) localToDistributed.chpl:11
lhsBaseType: [BlockDom(1,int(64),one,unmanaged DefaultDist)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

ArrayViewElision supported localToDistributed.chpl:24
ArrayViewElision supported (dynamic locality check required) localToDistributed.chpl:24
lhsBaseType: [CyclicDom(1,int(64),one)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

ArrayViewElision supported localToDistributed.chpl:35
ArrayViewElision supported (dynamic locality check required) localToDistributed.chpl:35
lhsBaseType: [StencilDom(1,int(64),one,false)] int(64)
lhsIndexingExprs:
range(int(64),both,one)
rhsBaseType: [domain(1,int(64),one)] int(64)
rhsIndexingExprs:
range(int(64),both,one)

operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
In DefaultRectangular._simpleTransfer(): Alo=(0,), Blo=(0,), len=1, elemSize=8
operator =(a:[],b:[]): successfully completed bulk transfer
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
In DefaultRectangular._simpleTransfer(): Alo=(0,), Blo=(0,), len=1, elemSize=8
operator =(a:[],b:[]): successfully completed bulk transfer
Performing protoSlice bulk transfer
operator =(a:[],b:[]): in chpl__bulkTransferArray
operator =(a:[],b:[]): attempting doiBulkTransferFromKnown
Expand Down