[PATCH] D86360: Add new hidden option -print-changed which only reports changes to IR

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 31 21:24:10 PDT 2020


aeubanks added inline comments.


================
Comment at: llvm/test/Other/change-printer.ll:16
+; are checked.
+; N.B. As passes change functionality, this test may fail because it reports
+; whether a pass made a change or not, which can change as pass functionality
----------------
jamieschmeiser wrote:
> jamieschmeiser wrote:
> > jamieschmeiser wrote:
> > > ychen wrote:
> > > > It has some non-trivial maintenance costs. I would suggest to use unit test for this patch altogether.
> > > I'm sorry but I do not understand what you mean.
> > > I tried to choose 2 passes that currently and would likely continue to make changes to the source in the test and the tests only check that changes are made and not what the changes actually are, to avoid churn.  I think the only way to completely avoid the possibility of churn would be to not test it.
> > I now know what you mean and will look into the possibility.
> > 
> I do not think it is possible to capture the output from dbgs() within the unit test framework.  The only information I could find about capturing std::out or std::err involved using non-public APIs in the test framework.  Also, llvm::dbgs() does not actually use std::out but uses its own buffer.  This buffer can be sized but that would require altering the hidden option controlling the size to make it non-static so that it could be accessed within the test, which doesn't seem like a good plan.  I think these tests are the best that can be done and if the concern is that they are fragile, then I will remove them if you prefer, seeing as this is for debugging information anyway.
The IR seems a bit heavy. Could you do something simple like
```
define i32 @f() {
  %a = add i32 2, 3
  ret i32 %a
}
```
and run
`opt -passes='no-op-function,instsimplify'`?
`no-op-function` serves as `make-no-change` and `instsimplify` will simplify the IR to just `ret i32 5`, serving as `make-a-change`.
That way there's no need to add two more passes to LLVM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86360/new/

https://reviews.llvm.org/D86360



More information about the llvm-commits mailing list