leonardchan added a comment. In D63638#1560991 <https://reviews.llvm.org/D63638#1560991>, @craig.topper wrote:
> In D63638#1560846 <https://reviews.llvm.org/D63638#1560846>, @spatel wrote: > > > I skimmed D63174 <https://reviews.llvm.org/D63174> but haven't applied > > either of these patches to test locally, so I may not have the full picture. > > > > IMO, we do not want clang regression tests running > > -instcombine/-instsimplify. That can cause clang tests to break when an > > underlying LLVM change is made. Forcing LLVM devs to depend on clang and > > fix the resulting breakage is backwards and unexpected extra work. This has > > happened to me several times. > > > > As a compromise to the -O0 IR explosion, we do have precedent for running > > the optimizer's -mem2reg pass since that doesn't change frequently at this > > point. > > > > And I haven't tried this, but we do have utils/update_cc_test_checks.py - > > this is supposed to take the manual labor out of generating assertions in > > the same way that we do in the optimizer and codegen regression tests with > > utils/update_test_checks.py and utils/update_llc_test_checks.py. Can you > > start with that and remove the irrelevant CHECK lines, so only the > > common/important lines remain? Or just use independent FileCheck > > '--check-prefixes'? > > > I definitely agree running -instcombine would be bad since it can replace > squences with other sequences. -instsimplify is a little less scary because > our intrinsic tests shouldn't really have a lot of things that are trivially > reducible. Though that may not be as true as I want it to be. The main issue > we seemed to need -instsimplify for with the new pass manager is to merge > redundant bitcasts. The inliner in the old pass manager seemed to do that > itself, but the new pass manager's always inliner doesn't. I think it could be that the new PM Inliner isn't added to the pipeline at -O0. It only seems to be added during optimized runs. @chandlerc might know if this was intentional or not. If so, perhaps these bitcasts are intended and the new PM is still doing its job in this case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63638/new/ https://reviews.llvm.org/D63638 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
