Page MenuHomePhabricator

[ConstraintElimination] Change debug output to display variable names.

Authored by zjaffal on Jan 26 2023, 5:34 AM.



Previously when constraint system outputs the rows in the system the variables used are x1,2...n making it hard to infer which ones they relate to in the IR

Diff Detail

Event Timeline

zjaffal created this revision.Jan 26 2023, 5:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2023, 5:34 AM
zjaffal requested review of this revision.Jan 26 2023, 5:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2023, 5:34 AM

Thanks for looking into this! Would it make sense to change ConstraintSystem to hold a pointer/reference to the corresponding variable map and then isolate the name generation logic and dumping to ConstraintSystem.cpp only?

nikic added a comment.Jan 26 2023, 5:43 AM

We should make sure that no name map is generated when not debugging.

zjaffal updated this revision to Diff 492488.Jan 26 2023, 9:29 AM

Refactored the printing logic to be done in ConstraintSystem

fhahn added a comment.Feb 6 2023, 2:18 PM

Would it be possible to add a test to show the additional dbg output, e.g. llvm/test/Transforms/ConstraintElimination/debug.ll?


capitalize first letter of sentence and period at end of sentence. Newline before comment


Please run clang-format and remove the needed ;


Pass argument as const reference?


Can use (auto &[V, Index] : Value2Index).


This function isn't needed any longer.


Change to const reference and adjust name?

zjaffal updated this revision to Diff 495845.Feb 8 2023, 7:27 AM
zjaffal marked 6 inline comments as done.
  1. Modify debug.ll test to display variable names.
  2. Delete unused functions and rename dumpdumpWithNames to dumpConstraint
  3. Address style comments
fhahn added inline comments.Feb 8 2023, 7:53 AM

still needs a newline before the comment.


not just const, const reference, i.e. const DenseMap<Value *, unsigned> &Value2Index


newline before the next function.


; here also needs removed and reformatting. Please run clang-format.


The bit about using Names needs to be removed.

zjaffal marked 5 inline comments as done.Feb 9 2023, 3:56 AM
zjaffal updated this revision to Diff 496079.Feb 9 2023, 3:57 AM

fix formatting and update dump() function comment

fhahn accepted this revision.Feb 10 2023, 7:44 AM

LGTM, with a few small nits addressed.


nit: from *the* Value2Index map.


nit: add newline here.


nit: the coding style recommends using auto sparingly and spell out full types unless they are too big. So here it would probably be better to use SmallVector<std::string>

This revision is now accepted and ready to land.Feb 10 2023, 7:44 AM
zjaffal updated this revision to Diff 497666.Feb 15 2023, 7:05 AM
zjaffal marked an inline comment as done.

Address nit comments

zjaffal marked 2 inline comments as done.Feb 15 2023, 7:06 AM
This revision was landed with ongoing or failed builds.Feb 15 2023, 7:08 AM
This revision was automatically updated to reflect the committed changes.