Page MenuHomePhabricator

[DA] Improve dump to show source and sink of the dependence
ClosedPublic

Authored by bmahjour on Dec 5 2019, 1:12 PM.

Details

Summary

The current da printer shows the dependence without indicating which instructions are being considered as the src vs dst. It also silently ignores call instructions, despite the fact that they create confused dependence edges to other memory instructions. This patch addresses these two issues plus a couple of minor non-functional improvements.

Diff Detail

Event Timeline

bmahjour created this revision.Dec 5 2019, 1:12 PM
fhahn added a subscriber: fhahn.Dec 5 2019, 1:22 PM
fhahn added inline comments.
llvm/lib/Analysis/DependenceAnalysis.cpp
199

Printing that last seems a bit odd to me. How about moving it up and have something like "da analyze - Src" << *SrcI << " --> Dst :" << *DstI?

llvm/test/Analysis/DependenceAnalysis/Dump.ll
20

Please check the complete output (should not be too much in the case) and it helps with getting the complete picture of the changed output.

bmahjour updated this revision to Diff 232451.Dec 5 2019, 2:14 PM
bmahjour marked 2 inline comments as done.

Check the complete da print output in the LIT test as per Florian's comment.

llvm/lib/Analysis/DependenceAnalysis.cpp
199

I thought about putting it on the same line as "da analyze", but that would break every test case in da, and also makes the output a bit cluttered and hard to read (it would be hard to find the actual dependence info).

I could put it at the beginning and indent the "da analyze - " part. ie change the output from:

'Dependence Analysis' for function 'foo':
da analyze - none!
  Src:  store float %conv, float* %arrayidx, align 4 --> Dst:  store float %conv, float* %arrayidx, align 4
da analyze - confused!
  Src:  store float %conv, float* %arrayidx, align 4 --> Dst:  call void @bar(float* %A)
da analyze - confused!
  Src:  call void @bar(float* %A) --> Dst:  call void @bar(float* %A)

to

'Dependence Analysis' for function 'foo':
Src:  store float %conv, float* %arrayidx, align 4 --> Dst:  store float %conv, float* %arrayidx, align 4
  da analyze - none!
Src:  store float %conv, float* %arrayidx, align 4 --> Dst:  call void @bar(float* %A)
  da analyze - confused!
Src:  call void @bar(float* %A) --> Dst:  call void @bar(float* %A)
  da analyze - confused!

Do people prefer that?

Nice. I always found this a bit awkward.

llvm/lib/Analysis/DependenceAnalysis.cpp
199

I would say the second probably makes more sense for people unfamiliar with the output before. It shows what it is analysing, then the result it came up with.

bmahjour updated this revision to Diff 233097.Dec 10 2019, 7:32 AM
bmahjour marked 3 inline comments as done.

Addressed review comments.

llvm/lib/Analysis/DependenceAnalysis.cpp
199

Sure. I've made it print the second way.

dmgreen accepted this revision.EditedDec 11 2019, 1:14 AM

Thanks. Looks like an good improvement to me. I'm happy if @fhahn is happy.

This revision is now accepted and ready to land.Dec 11 2019, 1:14 AM
fhahn accepted this revision.Dec 11 2019, 1:24 AM

LGTM thanks! I agree that putting analyzed instructions before the result looks good!

Thank you @dmgreen and @fhahn for the quick review.

This revision was automatically updated to reflect the committed changes.