This is an archive of the discontinued LLVM Phabricator instance.

[LoopRotate] Update dbg.value calls
ClosedPublic

Authored by samparker on Feb 21 2017, 2:54 AM.

Details

Summary

Propagate debug info through the newly inserted PHI nodes, this was previously attempted in D29578 at the SSA level. As an example of the issue this is fixing:

int main(int a) {

for(int i = 1; i < 10; i++) {
  func2(i, i+a);
}

}

If we currently compile with clang, loop rotation will transform the debug information so that the value 'i' is always set to the constant 1 in the body of the loop.

Diff Detail

Event Timeline

samparker created this revision.Feb 21 2017, 2:54 AM
samparker edited the summary of this revision. (Show Details)Feb 27 2017, 1:31 AM
aprantl added inline comments.Feb 27 2017, 1:40 PM
lib/Transforms/Scalar/LoopRotation.cpp
182

'.' at the end :-)

183

clang-format, range-based-for?

197

this is doing the lookup twice if successful. Maybe better to use find() instead?

samparker updated this revision to Diff 90004.Feb 28 2017, 3:29 AM

Updated to use range based loops and use find to reduce map lookup overhead.

Ping. @aprantl would you mind reviewing this? Thanks.

aprantl accepted this revision.Mar 6 2017, 8:41 AM

Seems generally plausible. LGTM with all inline comments addressed.

lib/Transforms/Scalar/LoopRotation.cpp
178

Technically LLVM coding guidelines want this to start with a lower case character. (but I see that other functions in the same file also violate this...)

178

Comment what the function is doing?

186

DbgValueMap.insert({DbgII->getVariableLocation(), DbgII)});

191

s/call/instrinsic/

198
  1. auto
  2. Why do you need a dyn_cast here? I would have either expected a static_cast, or something that actually checks the result.
390

call -> intrinsic

test/Transforms/LoopRotate/phi-dbgvalue.ll
3

Comment what the test is testing.

This revision is now accepted and ready to land.Mar 6 2017, 8:41 AM
This revision was automatically updated to reflect the committed changes.