[PATCH] D89500: Fix the error message with -fbasic-block-sections=list=<filename>

[David Blaikie via cfe-commits]

On Wed, Nov 4, 2020 at 9:07 PM Fāng-ruì Sòng <maskray at google.com> wrote:
>
> It is rare to report an error in BackendUtil.cpp . So I checked the
> other Diags.Report instance and noticed that -split-dwarf-file a.dwo
> -split-dwarf-output a.dwo (when a.dwo is not writable) suppresses the
> output. So there is no reason that -fbasic-block-sections=list= should
> not follow the convention.

Ah, thanks - I'm starting to see the connection, but it's still a few
extra steps for me. How'd you get from looking for Diags.Reports to
the split-dwarf-file case? Oh, because that produces an error (here:
https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/BackendUtil.cpp#L140
) & that error path suppresses output of any output files, not just
the one that couldn't be written to - right right.

Thanks for all the details!

Leaving the code review for now for Sri to take a look at. (good to
get some cross-pollination of reviewers/understanding of the issues in
the original code, etc)

>
> On Wed, Nov 4, 2020 at 8:18 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Wed, Nov 4, 2020 at 8:08 PM Fāng-ruì Sòng <maskray at google.com> wrote:
> >>
> >> I checked chmod -w a.dwo; clang -cc1 -debug-info-kind=limited
> >> -dwarf-version=4 -split-dwarf-file a.dwo -split-dwarf-output a.dwo
> >> -emit-obj -o - split-debug-output.c
> >> which suppresses the output, so -fbasic-block-sections=list= should
> >> follow the convention as well.
> >
> >
> > I missed a step as to the inference between the split-dwarf example and the fbasic-block-sections example. Could you explain further what the split-dwarf test was intending to demonstrate/how it relates to the -fbasic-block-sections example?
> >
> >>
> >>
> >> Sent https://reviews.llvm.org/D90815
> >>
> >> On Wed, Nov 4, 2020 at 7:26 PM David Blaikie <dblaikie at gmail.com> wrote:
> >> >
> >> >
> >> >
> >> > On Tue, Oct 27, 2020 at 2:21 PM Sriraman Tallam via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> >> >>
> >> >>
> >> >>
> >> >> On Tue, Oct 27, 2020 at 2:14 PM David Blaikie via Phabricator <reviews at reviews.llvm.org> wrote:
> >> >>>
> >> >>> dblaikie added a comment.
> >> >>>
> >> >>> @tmsriram ping on the follow-up here
> >> >>
> >> >>
> >> >> I checked in the patch that emits llvm instead of obj which spews garbage to the terminal as I wasn't redirecting it to /dev/null.  The test seems stable. Is there a particular concern? Sorry if I missed somethig here?
> >> >
> >> >
> >> > Oh, sorry - I missed your emails on-list, as they didn't end up on the review when viewed via Phabricator - that's most of the confusion. My mistake.
> >> >
> >> > Going back over it though - Yep, I totally missed the "ERROR" check line at the end (maybe worth an empty line between it and the UNIQUE check lines - as there's a break between UNIQUE and other lines (maybe the BB_* ones could use breaks too)).
> >> >
> >> > Though I'm still curious: Why is this command producing any object/binary output if it has produced an error message? That seems incorrect to me (generally if there's been any error, there wouldn't be output).
> >>
> >>
> >>
> >> --
> >> 宋方睿
>
>
>
> --
> 宋方睿