[PATCH] D125303: [update_llc_test_checks] Handle mixed asm and ISel debug output

Sat May 14 14:30:16 PDT 2022

arichardson added a comment.

In D125303#3506720 <https://reviews.llvm.org/D125303#3506720>, @ningxinr wrote:

> Thank you so much for the improvements, Alexander! It would be nice to support isel and asm at the same time. Although I do have a few questions and one concern with regard to an ongoing change of mine.
>
>> I noticed a potentially uninitialized variable warning after D119368 <https://reviews.llvm.org/D119368>.
>
> Would you mind pointing me to that warning? Is it complaining about `output_type`? And after your change the warning is gone? That does sound quite false positive-ish...
>
>> I have deleted the add_checks() function from isel.py as it is identical to the one in asm.py
>
> Please don't delete that `add_checks()` function yet. It is only identical to the one in `asm.py` because we are still working on it... I have a review D122824 <https://reviews.llvm.org/D122824> in progress (Sorry this one took a bit too long for me to get back to it, but I do plan to get back to it in the next few days.)

Ah I didn't see that one. For mixed ISel and non-ISel output this will need a different approach though since the correct prefix depends on the current RUN: line.

>> restored the old name (which fixes the formatting too)
>
> Did the warning complained about the naming as well? I am quite confused here. Is it the fact that this line was using `output_type` that caused the warning or the function name `add_checks()` has cause the warning? What is the motivation for changing back the name here? Is it because the `add_checks()` in `isel.py` is removed so we prefer the old name?

No warning here, I just noticed that the function call line continuations were no longer aligned after you renamed the function. I'll make a suggestion on that review.

> Thanks for the improvement of the feature and thanks  in advance for your patience with all my questions. :)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125303