Page MenuHomePhabricator

[OpenMPOpt] ICV Tracking Between BasicBlocks
AbandonedPublic

Authored by sstefan1 on Jul 31 2020, 3:17 PM.

Details

Summary

Calculate replacement values during update. Only use manifest for actual
deduplication. This also adds additional capabilities of tracking ICV
values across multiple BBs. Until now, we only replaced within the same BB.

Diff Detail

Unit TestsFailed

TimeTest
170 mslinux > Polly.ScopInfo::memcpy-raw-source.ll
Script: -- : 'RUN: at line 1'; opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo -polly-codegen-verify -basic-aa -scoped-noalias -tbaa -polly-scops -analyze < /mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/memcpy-raw-source.ll

Event Timeline

sstefan1 created this revision.Jul 31 2020, 3:17 PM
sstefan1 requested review of this revision.Jul 31 2020, 3:17 PM

This is still "intra-procedural", correct? Or can it replace across function boundaries too?

llvm/test/Transforms/OpenMP/icv_tracking.ll
19

Maybe rename them to indicate if it is a unknown_callee or a benign_callee.

This is still "intra-procedural", correct? Or can it replace across function boundaries too?

We are now able to check if call changes an ICV through function attribute. Follow up should make it possible to get an ICV value for a call.

llvm/test/Transforms/OpenMP/icv_tracking.ll
19

I think that name is more appropriate for @use() since we can't know if a declaration changes an ICV, right?

@use1() is not a great name, I agree with that. I'll change it.

Follow up with the interprocedural replacement sounds good.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1255

!empty()?

1295

Don't we want
CallInst *CI = OpenMPOpt::getCallIfRegularCall(U);
and a check for CI?

1299

can we have an assert to verify that the getter is either not mapped or mapped to ReplVal. If that triggers it would be bad since this is not monotonic anymore and could just spin.

1344

Does this work if the value was not "tracked", so no setter was seen, but "maybe written", by another unkown call?

I guess we need a test where we don't call use1 or use but something that then calls use.

llvm/test/Transforms/OpenMP/icv_tracking.ll
19

I meant the first is for use the second for use1. Having use is not too bad as I would assume the worst about it, but use1 really doesn't capture it is pure.

sstefan1 updated this revision to Diff 282478.Aug 2 2020, 1:31 PM

addressing comments

sstefan1 added inline comments.Aug 2 2020, 1:41 PM
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
1255

Yes, I inverted the check as well.

1344

You are right, this actually won't work.

I'll fix this and add a test.

sstefan1 updated this revision to Diff 282694.Aug 3 2020, 12:01 PM

cover "maybe written" by another unkown call case and add test for that.

sstefan1 abandoned this revision.Aug 12 2020, 10:01 AM

Good parts of this are merged in D85544.