[RFC] Should `clang -help` be refined/improved?

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
Hi all,

We are in the process of adding Flang-specific options to libclangDriver
and want to make sure that we don't pollute `clang -help` or `flang
-help` with irrelevant options.

# *PROBLEM IN GENERAL*
I have been looking at clang/include/Driver/Options.td (and various
files that define/modify/print compiler driver options) and I am not
sure what the expected implementation and behaviour should be. For
example, looking at ClangFlags [1], should `clang -help` display options
flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be reserved
for `clang -cc1 -help` and `clang -cc1as -help`, respectively?

What about `DriverOption` and `NoDriverOption`? Based on the
descriptions in Options.td [2], I would assume that only one of these
flags is be needed. Also, the current implementation of libclangDriver
seem to focus on `clang -help` vs `clang-cl -help` split [3]. Lastly,
`clang -help` prints a lot of options - should all of these be printed?

I'm bringing this up because we want to add Flang-specific options (i.e.
an option that's printed by `flang -help`, but not printed by `clang
-help`), and that's proving quite tricky. To this end, we recently have
submitted a patch that adds yet another item in ClangFlags (see
`NoClangOption` in [4], still in review!). However, it feels that if
libclangDriver was stricter about filtering the options (based on the
flags), we could avoid that.

# *PROBLEM IN DETAIL*
IIUC, libclangDriver should clearly define what flags should be
included/excluded when passing `-help` to `clang` and other tools.
Currently this is a bit vague (from Driver.cpp [5]):

```
unsigned IncludedFlagsBitmask = 0;
```

It boils down to "print most of the options available". Wouldn't the
following, more explicit approach, be preferred (example for `clang`):

```
unsigned IncludedFlagsBitmask =
     options::DriverOption |
     options::NoArgumentUnused |
     options::CoreOption |
     options::LinkOption
```

? However, there's a problem with this approach too: `clang -help` will
stop working as the definition of `help` contains none of the above
flags [6]:

```
def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
FC1Option, FlangOption]>,
   HelpText<"Display available options">;
```

This can be fixed by e.g. adding `CoreOption` to the definition above.
But there will be more options with similar problem. There's also a
group of options that contain no flags at all (so it's hard to classify
them). What should happen with them?

# *LLVM's OPTTABLE*
Perhaps we should improve this in LLVM in OptTable.{h|cpp} instead? For
instance, what's the relation between FlagsToExclude and FlagsToInclude
[7]? Which one takes precedence? Note that any changes in LLVM's
OptTable will have impact beyond Clang and Flang, so I don't want dive
too deep into this in this RFC. But I suspect that refining that API a
bit could help Clang and Flang.

# *SUGGESTION*
I think that some of these issues could be resolved (or at least
improved) if:
  * all options defined in clang's Options.td were required to contain a
flag or otherwise be ignored
  * every flag clearly communicated the mode (e.g. compiler driver vs
frontend driver) in which particular option is available (i.e. there
should be no need for `DriverOption` _and_ `NoDriverOption`)
  * every tool (e.g. `clang` or `clang -cc1`) was more explicit about
the supported options (or, at least, options displayed when passing `-help`)

Do these suggestions make sense? Is my reasoning valid? Or did I get it
completely wrong? :)

Thanks for reading!
On behalf of the Arm Fortran team,
-Andrzej

[1]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
[2]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
[3]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
[4] https://reviews.llvm.org/D87989
[5]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
[6]
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
[7]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
+Hans

I think Hans and I are the ones most responsible for the behavior of clang-cl --help. For our part, we made sure that clang-cl --help prints something useful, but did our best to leave the rest of the existing mess alone. If you make changes, please check that the clang-cl --help output remains as useful as possible, since we have made an effort to curate that with help text.

I think any changes you propose in this area will probably be an improvement over the current situation.

I would encourage you to try to improve the LLVM Option library if you can, but obviously it's always not straightforward. Most of the other clients (some LLVM tools, LLD) are pretty simple and easy to update for new APIs.

On Wed, Oct 7, 2020 at 12:33 PM Andrzej Warzynski via cfe-dev <[hidden email]> wrote:
Hi all,

We are in the process of adding Flang-specific options to libclangDriver
and want to make sure that we don't pollute `clang -help` or `flang
-help` with irrelevant options.

# *PROBLEM IN GENERAL*
I have been looking at clang/include/Driver/Options.td (and various
files that define/modify/print compiler driver options) and I am not
sure what the expected implementation and behaviour should be. For
example, looking at ClangFlags [1], should `clang -help` display options
flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be reserved
for `clang -cc1 -help` and `clang -cc1as -help`, respectively?

What about `DriverOption` and `NoDriverOption`? Based on the
descriptions in Options.td [2], I would assume that only one of these
flags is be needed. Also, the current implementation of libclangDriver
seem to focus on `clang -help` vs `clang-cl -help` split [3]. Lastly,
`clang -help` prints a lot of options - should all of these be printed?

I'm bringing this up because we want to add Flang-specific options (i.e.
an option that's printed by `flang -help`, but not printed by `clang
-help`), and that's proving quite tricky. To this end, we recently have
submitted a patch that adds yet another item in ClangFlags (see
`NoClangOption` in [4], still in review!). However, it feels that if
libclangDriver was stricter about filtering the options (based on the
flags), we could avoid that.

# *PROBLEM IN DETAIL*
IIUC, libclangDriver should clearly define what flags should be
included/excluded when passing `-help` to `clang` and other tools.
Currently this is a bit vague (from Driver.cpp [5]):

```
unsigned IncludedFlagsBitmask = 0;
```

It boils down to "print most of the options available". Wouldn't the
following, more explicit approach, be preferred (example for `clang`):

```
unsigned IncludedFlagsBitmask =
     options::DriverOption |
     options::NoArgumentUnused |
     options::CoreOption |
     options::LinkOption
```

? However, there's a problem with this approach too: `clang -help` will
stop working as the definition of `help` contains none of the above
flags [6]:

```
def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
FC1Option, FlangOption]>,
   HelpText<"Display available options">;
```

This can be fixed by e.g. adding `CoreOption` to the definition above.
But there will be more options with similar problem. There's also a
group of options that contain no flags at all (so it's hard to classify
them). What should happen with them?

# *LLVM's OPTTABLE*
Perhaps we should improve this in LLVM in OptTable.{h|cpp} instead? For
instance, what's the relation between FlagsToExclude and FlagsToInclude
[7]? Which one takes precedence? Note that any changes in LLVM's
OptTable will have impact beyond Clang and Flang, so I don't want dive
too deep into this in this RFC. But I suspect that refining that API a
bit could help Clang and Flang.

# *SUGGESTION*
I think that some of these issues could be resolved (or at least
improved) if:
  * all options defined in clang's Options.td were required to contain a
flag or otherwise be ignored
  * every flag clearly communicated the mode (e.g. compiler driver vs
frontend driver) in which particular option is available (i.e. there
should be no need for `DriverOption` _and_ `NoDriverOption`)
  * every tool (e.g. `clang` or `clang -cc1`) was more explicit about
the supported options (or, at least, options displayed when passing `-help`)

Do these suggestions make sense? Is my reasoning valid? Or did I get it
completely wrong? :)

Thanks for reading!
On behalf of the Arm Fortran team,
-Andrzej

[1]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
[2]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
[3]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
[4] https://reviews.llvm.org/D87989
[5]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
[6]
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
[7]
https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
On Tue, Oct 13, 2020 at 12:29 PM Reid Kleckner via cfe-dev
<[hidden email]> wrote:

>
> +Hans
>
> I think Hans and I are the ones most responsible for the behavior of clang-cl --help. For our part, we made sure that clang-cl --help prints something useful, but did our best to leave the rest of the existing mess alone. If you make changes, please check that the clang-cl --help output remains as useful as possible, since we have made an effort to curate that with help text.
>
> I think any changes you propose in this area will probably be an improvement over the current situation.
>
> I would encourage you to try to improve the LLVM Option library if you can, but obviously it's always not straightforward. Most of the other clients (some LLVM tools, LLD) are pretty simple and easy to update for new APIs.
>
> On Wed, Oct 7, 2020 at 12:33 PM Andrzej Warzynski via cfe-dev <[hidden email]> wrote:
>>
>> Hi all,
>>
>> We are in the process of adding Flang-specific options to libclangDriver
>> and want to make sure that we don't pollute `clang -help` or `flang
>> -help` with irrelevant options.
>>
>> # *PROBLEM IN GENERAL*
>> I have been looking at clang/include/Driver/Options.td (and various
>> files that define/modify/print compiler driver options) and I am not
>> sure what the expected implementation and behaviour should be. For
>> example, looking at ClangFlags [1], should `clang -help` display options
>> flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be reserved
>> for `clang -cc1 -help` and `clang -cc1as -help`, respectively?
>>
>> What about `DriverOption` and `NoDriverOption`? Based on the
>> descriptions in Options.td [2], I would assume that only one of these
>> flags is be needed. Also, the current implementation of libclangDriver
>> seem to focus on `clang -help` vs `clang-cl -help` split [3]. Lastly,
>> `clang -help` prints a lot of options - should all of these be printed?

The use of DriverOption is very confusing. It served two purposes (1)
whether an option should be forwarded to gcc (for baremetal toolchain)
(2) whether an option should be forwarded for -Xarch -Xarch_host
-Xarch_device (macOS and CUDA use cases)

I have refactored (1) to use a new TableGen def 'LinkOption'. Now
DriverOption's only purpose is (2). CCed Artem who may know (2)

>> I'm bringing this up because we want to add Flang-specific options (i.e.
>> an option that's printed by `flang -help`, but not printed by `clang
>> -help`), and that's proving quite tricky. To this end, we recently have
>> submitted a patch that adds yet another item in ClangFlags (see
>> `NoClangOption` in [4], still in review!). However, it feels that if
>> libclangDriver was stricter about filtering the options (based on the
>> flags), we could avoid that.
>>
>> # *PROBLEM IN DETAIL*
>> IIUC, libclangDriver should clearly define what flags should be
>> included/excluded when passing `-help` to `clang` and other tools.
>> Currently this is a bit vague (from Driver.cpp [5]):
>>
>> ```
>> unsigned IncludedFlagsBitmask = 0;
>> ```
>>
>> It boils down to "print most of the options available". Wouldn't the
>> following, more explicit approach, be preferred (example for `clang`):
>>
>> ```
>> unsigned IncludedFlagsBitmask =
>>      options::DriverOption |
>>      options::NoArgumentUnused |
>>      options::CoreOption |
>>      options::LinkOption
>> ```
>>
>> ? However, there's a problem with this approach too: `clang -help` will
>> stop working as the definition of `help` contains none of the above
>> flags [6]:
>>
>> ```
>> def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
>> FC1Option, FlangOption]>,
>>    HelpText<"Display available options">;
>> ```
>>
>> This can be fixed by e.g. adding `CoreOption` to the definition above.
>> But there will be more options with similar problem. There's also a
>> group of options that contain no flags at all (so it's hard to classify
>> them). What should happen with them?
>>
>> # *LLVM's OPTTABLE*
>> Perhaps we should improve this in LLVM in OptTable.{h|cpp} instead? For
>> instance, what's the relation between FlagsToExclude and FlagsToInclude
>> [7]? Which one takes precedence? Note that any changes in LLVM's
>> OptTable will have impact beyond Clang and Flang, so I don't want dive
>> too deep into this in this RFC. But I suspect that refining that API a
>> bit could help Clang and Flang.

AFAIK the only users of OptTable are: clang (clang-cl), lld (lld-link,
ld.lld, wasm-ld), and llvm-symbolizer.
You can check whether their -help output changes with your refactoring.

>> # *SUGGESTION*
>> I think that some of these issues could be resolved (or at least
>> improved) if:
>>   * all options defined in clang's Options.td were required to contain a
>> flag or otherwise be ignored
>>   * every flag clearly communicated the mode (e.g. compiler driver vs
>> frontend driver) in which particular option is available (i.e. there
>> should be no need for `DriverOption` _and_ `NoDriverOption`)
>>   * every tool (e.g. `clang` or `clang -cc1`) was more explicit about
>> the supported options (or, at least, options displayed when passing `-help`)
>>
>> Do these suggestions make sense? Is my reasoning valid? Or did I get it
>> completely wrong? :)
>>
>> Thanks for reading!
>> On behalf of the Arm Fortran team,
>> -Andrzej
>>
>> [1]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
>> [2]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
>> [3]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
>> [4] https://reviews.llvm.org/D87989
>> [5]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
>> [6]
>> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
>> [7]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
>> _______________________________________________
>> cfe-dev mailing list
>> [hidden email]
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
llvm-lipo also uses OptTable.

On 10/13/20, 1:02 PM, "cfe-dev on behalf of Fāng-ruì Sòng via cfe-dev" <[hidden email] on behalf of [hidden email]> wrote:

    On Tue, Oct 13, 2020 at 12:29 PM Reid Kleckner via cfe-dev
    <[hidden email]> wrote:
    >
    > +Hans
    >
    > I think Hans and I are the ones most responsible for the behavior of clang-cl --help. For our part, we made sure that clang-cl --help prints something useful, but did our best to leave the rest of the existing mess alone. If you make changes, please check that the clang-cl --help output remains as useful as possible, since we have made an effort to curate that with help text.
    >
    > I think any changes you propose in this area will probably be an improvement over the current situation.
    >
    > I would encourage you to try to improve the LLVM Option library if you can, but obviously it's always not straightforward. Most of the other clients (some LLVM tools, LLD) are pretty simple and easy to update for new APIs.
    >
    > On Wed, Oct 7, 2020 at 12:33 PM Andrzej Warzynski via cfe-dev <[hidden email]> wrote:
    >>
    >> Hi all,
    >>
    >> We are in the process of adding Flang-specific options to libclangDriver
    >> and want to make sure that we don't pollute `clang -help` or `flang
    >> -help` with irrelevant options.
    >>
    >> # *PROBLEM IN GENERAL*
    >> I have been looking at clang/include/Driver/Options.td (and various
    >> files that define/modify/print compiler driver options) and I am not
    >> sure what the expected implementation and behaviour should be. For
    >> example, looking at ClangFlags [1], should `clang -help` display options
    >> flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be reserved
    >> for `clang -cc1 -help` and `clang -cc1as -help`, respectively?
    >>
    >> What about `DriverOption` and `NoDriverOption`? Based on the
    >> descriptions in Options.td [2], I would assume that only one of these
    >> flags is be needed. Also, the current implementation of libclangDriver
    >> seem to focus on `clang -help` vs `clang-cl -help` split [3]. Lastly,
    >> `clang -help` prints a lot of options - should all of these be printed?

    The use of DriverOption is very confusing. It served two purposes (1)
    whether an option should be forwarded to gcc (for baremetal toolchain)
    (2) whether an option should be forwarded for -Xarch -Xarch_host
    -Xarch_device (macOS and CUDA use cases)

    I have refactored (1) to use a new TableGen def 'LinkOption'. Now
    DriverOption's only purpose is (2). CCed Artem who may know (2)

    >> I'm bringing this up because we want to add Flang-specific options (i.e.
    >> an option that's printed by `flang -help`, but not printed by `clang
    >> -help`), and that's proving quite tricky. To this end, we recently have
    >> submitted a patch that adds yet another item in ClangFlags (see
    >> `NoClangOption` in [4], still in review!). However, it feels that if
    >> libclangDriver was stricter about filtering the options (based on the
    >> flags), we could avoid that.
    >>
    >> # *PROBLEM IN DETAIL*
    >> IIUC, libclangDriver should clearly define what flags should be
    >> included/excluded when passing `-help` to `clang` and other tools.
    >> Currently this is a bit vague (from Driver.cpp [5]):
    >>
    >> ```
    >> unsigned IncludedFlagsBitmask = 0;
    >> ```
    >>
    >> It boils down to "print most of the options available". Wouldn't the
    >> following, more explicit approach, be preferred (example for `clang`):
    >>
    >> ```
    >> unsigned IncludedFlagsBitmask =
    >>      options::DriverOption |
    >>      options::NoArgumentUnused |
    >>      options::CoreOption |
    >>      options::LinkOption
    >> ```
    >>
    >> ? However, there's a problem with this approach too: `clang -help` will
    >> stop working as the definition of `help` contains none of the above
    >> flags [6]:
    >>
    >> ```
    >> def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
    >> FC1Option, FlangOption]>,
    >>    HelpText<"Display available options">;
    >> ```
    >>
    >> This can be fixed by e.g. adding `CoreOption` to the definition above.
    >> But there will be more options with similar problem. There's also a
    >> group of options that contain no flags at all (so it's hard to classify
    >> them). What should happen with them?
    >>
    >> # *LLVM's OPTTABLE*
    >> Perhaps we should improve this in LLVM in OptTable.{h|cpp} instead? For
    >> instance, what's the relation between FlagsToExclude and FlagsToInclude
    >> [7]? Which one takes precedence? Note that any changes in LLVM's
    >> OptTable will have impact beyond Clang and Flang, so I don't want dive
    >> too deep into this in this RFC. But I suspect that refining that API a
    >> bit could help Clang and Flang.

    AFAIK the only users of OptTable are: clang (clang-cl), lld (lld-link,
    ld.lld, wasm-ld), and llvm-symbolizer.
    You can check whether their -help output changes with your refactoring.

    >> # *SUGGESTION*
    >> I think that some of these issues could be resolved (or at least
    >> improved) if:
    >>   * all options defined in clang's Options.td were required to contain a
    >> flag or otherwise be ignored
    >>   * every flag clearly communicated the mode (e.g. compiler driver vs
    >> frontend driver) in which particular option is available (i.e. there
    >> should be no need for `DriverOption` _and_ `NoDriverOption`)
    >>   * every tool (e.g. `clang` or `clang -cc1`) was more explicit about
    >> the supported options (or, at least, options displayed when passing `-help`)
    >>
    >> Do these suggestions make sense? Is my reasoning valid? Or did I get it
    >> completely wrong? :)
    >>
    >> Thanks for reading!
    >> On behalf of the Arm Fortran team,
    >> -Andrzej
    >>
    >> [1]
    >> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
    >> [2]
    >> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
    >> [3]
    >> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
    >> [4] https://reviews.llvm.org/D87989 
    >> [5]
    >> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
    >> [6]
    >> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
    >> [7]
    >> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
    >> _______________________________________________
    >> cfe-dev mailing list
    >> [hidden email]
    >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev 
    >
    > _______________________________________________
    > cfe-dev mailing list
    > [hidden email]
    > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev 



    --
    宋方睿
    _______________________________________________
    cfe-dev mailing list
    [hidden email]
    https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev 

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
In reply to this post by Manas via cfe-dev


On Tue, Oct 13, 2020 at 1:01 PM Fāng-ruì Sòng <[hidden email]> wrote:
On Tue, Oct 13, 2020 at 12:29 PM Reid Kleckner via cfe-dev
<[hidden email]> wrote:
>
> +Hans
>
> I think Hans and I are the ones most responsible for the behavior of clang-cl --help. For our part, we made sure that clang-cl --help prints something useful, but did our best to leave the rest of the existing mess alone. If you make changes, please check that the clang-cl --help output remains as useful as possible, since we have made an effort to curate that with help text.
>
> I think any changes you propose in this area will probably be an improvement over the current situation.
>
> I would encourage you to try to improve the LLVM Option library if you can, but obviously it's always not straightforward. Most of the other clients (some LLVM tools, LLD) are pretty simple and easy to update for new APIs.
>
> On Wed, Oct 7, 2020 at 12:33 PM Andrzej Warzynski via cfe-dev <[hidden email]> wrote:
>>
>> Hi all,
>>
>> We are in the process of adding Flang-specific options to libclangDriver
>> and want to make sure that we don't pollute `clang -help` or `flang
>> -help` with irrelevant options.
>>
>> # *PROBLEM IN GENERAL*
>> I have been looking at clang/include/Driver/Options.td (and various
>> files that define/modify/print compiler driver options) and I am not
>> sure what the expected implementation and behaviour should be. For
>> example, looking at ClangFlags [1], should `clang -help` display options
>> flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be reserved
>> for `clang -cc1 -help` and `clang -cc1as -help`, respectively?
>>
>> What about `DriverOption` and `NoDriverOption`? Based on the
>> descriptions in Options.td [2], I would assume that only one of these
>> flags is be needed. Also, the current implementation of libclangDriver
>> seem to focus on `clang -help` vs `clang-cl -help` split [3]. Lastly,
>> `clang -help` prints a lot of options - should all of these be printed?

The use of DriverOption is very confusing. It served two purposes (1)
whether an option should be forwarded to gcc (for baremetal toolchain)
(2) whether an option should be forwarded for -Xarch -Xarch_host
-Xarch_device (macOS and CUDA use cases)

I have refactored (1) to use a new TableGen def 'LinkOption'. Now
DriverOption's only purpose is (2). CCed Artem who may know (2)

`-Xarch` and `-Xarch_host` could use some improvement. A lot of it. Last time I tried it couldn't even pass options with arguments via that mechanism.

Long time ago echristo@ and I talked about making it much more generic.
The rough idea was that instead of each target parsing their own -Xarch, teach option parser that some options only apply under some conditions.
E.g. If user passed `-foo=1 -bar=2 -Xarch=A1 -foo=3 -optA1 -Xarch=host -bar=4 -Xarch=all -buz`,
Host compilation would see: `-foo=1 -bar=4 -buz`
target A1 would see `-foo=1 -foo=3 -optA1 -buz`

It could be further specialized to provide something like `-Xarch=gpu` to specify options that should apply to all GPU-side compilations.
Some options could be marked as implicitly top or host only (e.g. we don't want sanitizer options to propagate to GPU sub-compilation).

It didn't go past the whiteboard sketches, though.

--Artem

 

>> I'm bringing this up because we want to add Flang-specific options (i.e.
>> an option that's printed by `flang -help`, but not printed by `clang
>> -help`), and that's proving quite tricky. To this end, we recently have
>> submitted a patch that adds yet another item in ClangFlags (see
>> `NoClangOption` in [4], still in review!). However, it feels that if
>> libclangDriver was stricter about filtering the options (based on the
>> flags), we could avoid that.
>>
>> # *PROBLEM IN DETAIL*
>> IIUC, libclangDriver should clearly define what flags should be
>> included/excluded when passing `-help` to `clang` and other tools.
>> Currently this is a bit vague (from Driver.cpp [5]):
>>
>> ```
>> unsigned IncludedFlagsBitmask = 0;
>> ```
>>
>> It boils down to "print most of the options available". Wouldn't the
>> following, more explicit approach, be preferred (example for `clang`):
>>
>> ```
>> unsigned IncludedFlagsBitmask =
>>      options::DriverOption |
>>      options::NoArgumentUnused |
>>      options::CoreOption |
>>      options::LinkOption
>> ```
>>
>> ? However, there's a problem with this approach too: `clang -help` will
>> stop working as the definition of `help` contains none of the above
>> flags [6]:
>>
>> ```
>> def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
>> FC1Option, FlangOption]>,
>>    HelpText<"Display available options">;
>> ```
>>
>> This can be fixed by e.g. adding `CoreOption` to the definition above.
>> But there will be more options with similar problem. There's also a
>> group of options that contain no flags at all (so it's hard to classify
>> them). What should happen with them?
>>
>> # *LLVM's OPTTABLE*
>> Perhaps we should improve this in LLVM in OptTable.{h|cpp} instead? For
>> instance, what's the relation between FlagsToExclude and FlagsToInclude
>> [7]? Which one takes precedence? Note that any changes in LLVM's
>> OptTable will have impact beyond Clang and Flang, so I don't want dive
>> too deep into this in this RFC. But I suspect that refining that API a
>> bit could help Clang and Flang.

AFAIK the only users of OptTable are: clang (clang-cl), lld (lld-link,
ld.lld, wasm-ld), and llvm-symbolizer.
You can check whether their -help output changes with your refactoring.

>> # *SUGGESTION*
>> I think that some of these issues could be resolved (or at least
>> improved) if:
>>   * all options defined in clang's Options.td were required to contain a
>> flag or otherwise be ignored
>>   * every flag clearly communicated the mode (e.g. compiler driver vs
>> frontend driver) in which particular option is available (i.e. there
>> should be no need for `DriverOption` _and_ `NoDriverOption`)
>>   * every tool (e.g. `clang` or `clang -cc1`) was more explicit about
>> the supported options (or, at least, options displayed when passing `-help`)
>>
>> Do these suggestions make sense? Is my reasoning valid? Or did I get it
>> completely wrong? :)
>>
>> Thanks for reading!
>> On behalf of the Arm Fortran team,
>> -Andrzej
>>
>> [1]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
>> [2]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
>> [3]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
>> [4] https://reviews.llvm.org/D87989
>> [5]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
>> [6]
>> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
>> [7]
>> https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
>> _______________________________________________
>> cfe-dev mailing list
>> [hidden email]
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev



--
宋方睿


--
--Artem Belevich

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Should `clang -help` be refined/improved?

Manas via cfe-dev
Thank you all for your comments, this is very helpful!

Overall I think that there's agreement that the output of `clang -help`
and other tools should be refined and that we need a more explicit
mechanisms for defining what is printed and what isn't when using `-help`.

The approach proposed for Flang (i.e. add a new flag in
clang::driver::options::ClangFlags for Flang-only options [3]) is
consistent with the current implementation and the proposed
improvements. I assume that there's no need to discuss this further.

Below is a quick summary of the proposed changes. These changes are
rather straightforward, but are likely to change the output of `-help`
for various tools. I think that clang (clang-cl, `clang -cc1`) will be
most affected. I also need to look into lld (lld-link, ld.lld, wasm-ld),
llvm-symbolizer and llvm-lipo.

I've only had a chance to prepare one patch [1]. I need to experiment a
bit more before submitting other changes.

# libclangDriver: DriverOption
This seems like a low-hanging fruit. Based on your input I suggest that
we rename DriverOption to NoXarchOption [1]. That's more consistent with
the actual usage within Clang [2]. Hopefully the new name will make
things clearer for our future selves. Please review the patch [1]. Your
feedback is much appreciated!

# LLVM: OptTable
I've already experimented with `FlagsToExclude` and `FlagsToInclude` and
I think that we could get rid of one of them. Their usage is rather
limited, so this shouldn't be too intrusive.

Perhaps it would be good to have a mechanism to mark an option as
`RequiresAtMostOneFlag`? Such option would be required to have exactly
only flag. If that flag is not added to `FlagsToInclude`, then the
corresponding option is not visible/printed/available. AFAIK, currently
that's not possible.

# libclangDriver: Strict definition of what is printed with `-help`
For _all_ tools that use libclangDriver, `-help` should be based on a
strict definition of what is printed and what isn't, e.g. for `clang`:
```
unsigned IncludedFlagsBitmask =
     options::DriverOption |
     options::NoArgumentUnused |
     options::CoreOption |
     options::LinkOption
```
Anything that's not on that list would not be printed. This would be a
straightforward change, but fine-tuning `-help` will be hard. Flags for
every option in Options.td would have to revisited.

-Andrzej

[1] https://reviews.llvm.org/D89799
[2]
https://github.com/llvm/llvm-project/blob/691eb814c1ae38d5015bf070dfed3fd54d542582/clang/lib/Driver/ToolChain.cpp#L1201-L1204
[3] https://reviews.llvm.org/D87989

On 13/10/2020 21:47, Artem Belevich wrote:

>
>
> On Tue, Oct 13, 2020 at 1:01 PM Fāng-ruì Sòng <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     On Tue, Oct 13, 2020 at 12:29 PM Reid Kleckner via cfe-dev
>     <[hidden email] <mailto:[hidden email]>> wrote:
>      >
>      > +Hans
>      >
>      > I think Hans and I are the ones most responsible for the behavior
>     of clang-cl --help. For our part, we made sure that clang-cl --help
>     prints something useful, but did our best to leave the rest of the
>     existing mess alone. If you make changes, please check that the
>     clang-cl --help output remains as useful as possible, since we have
>     made an effort to curate that with help text.
>      >
>      > I think any changes you propose in this area will probably be an
>     improvement over the current situation.
>      >
>      > I would encourage you to try to improve the LLVM Option library
>     if you can, but obviously it's always not straightforward. Most of
>     the other clients (some LLVM tools, LLD) are pretty simple and easy
>     to update for new APIs.
>      >
>      > On Wed, Oct 7, 2020 at 12:33 PM Andrzej Warzynski via cfe-dev
>     <[hidden email] <mailto:[hidden email]>> wrote:
>      >>
>      >> Hi all,
>      >>
>      >> We are in the process of adding Flang-specific options to
>     libclangDriver
>      >> and want to make sure that we don't pollute `clang -help` or `flang
>      >> -help` with irrelevant options.
>      >>
>      >> # *PROBLEM IN GENERAL*
>      >> I have been looking at clang/include/Driver/Options.td (and various
>      >> files that define/modify/print compiler driver options) and I am not
>      >> sure what the expected implementation and behaviour should be. For
>      >> example, looking at ClangFlags [1], should `clang -help` display
>     options
>      >> flagged as `CC1Option` and `CC1AsOption`?  Shouldn't these be
>     reserved
>      >> for `clang -cc1 -help` and `clang -cc1as -help`, respectively?
>      >>
>      >> What about `DriverOption` and `NoDriverOption`? Based on the
>      >> descriptions in Options.td [2], I would assume that only one of
>     these
>      >> flags is be needed. Also, the current implementation of
>     libclangDriver
>      >> seem to focus on `clang -help` vs `clang-cl -help` split [3].
>     Lastly,
>      >> `clang -help` prints a lot of options - should all of these be
>     printed?
>
>     The use of DriverOption is very confusing. It served two purposes (1)
>     whether an option should be forwarded to gcc (for baremetal toolchain)
>     (2) whether an option should be forwarded for -Xarch -Xarch_host
>     -Xarch_device (macOS and CUDA use cases)
>
>     I have refactored (1) to use a new TableGen def 'LinkOption'. Now
>     DriverOption's only purpose is (2). CCed Artem who may know (2)
>
>
> `-Xarch` and `-Xarch_host` could use some improvement. A lot of it. Last
> time I tried it couldn't even pass options with arguments via that
> mechanism.
>
> Long time ago echristo@ and I talked about making it much more generic.
> The rough idea was that instead of each target parsing their own -Xarch,
> teach option parser that some options only apply under some conditions.
> E.g. If user passed `-foo=1 -bar=2 -Xarch=A1 -foo=3 -optA1 -Xarch=host
> -bar=4 -Xarch=all -buz`,
> Host compilation would see: `-foo=1 -bar=4 -buz`
> target A1 would see `-foo=1 -foo=3 -optA1 -buz`
>
> It could be further specialized to provide something like `-Xarch=gpu`
> to specify options that should apply to all GPU-side compilations.
> Some options could be marked as implicitly top or host only (e.g. we
> don't want sanitizer options to propagate to GPU sub-compilation).
>
> It didn't go past the whiteboard sketches, though.
>
> --Artem
>
>
>
>      >> I'm bringing this up because we want to add Flang-specific
>     options (i.e.
>      >> an option that's printed by `flang -help`, but not printed by `clang
>      >> -help`), and that's proving quite tricky. To this end, we
>     recently have
>      >> submitted a patch that adds yet another item in ClangFlags (see
>      >> `NoClangOption` in [4], still in review!). However, it feels that if
>      >> libclangDriver was stricter about filtering the options (based
>     on the
>      >> flags), we could avoid that.
>      >>
>      >> # *PROBLEM IN DETAIL*
>      >> IIUC, libclangDriver should clearly define what flags should be
>      >> included/excluded when passing `-help` to `clang` and other tools.
>      >> Currently this is a bit vague (from Driver.cpp [5]):
>      >>
>      >> ```
>      >> unsigned IncludedFlagsBitmask = 0;
>      >> ```
>      >>
>      >> It boils down to "print most of the options available". Wouldn't the
>      >> following, more explicit approach, be preferred (example for
>     `clang`):
>      >>
>      >> ```
>      >> unsigned IncludedFlagsBitmask =
>      >>      options::DriverOption |
>      >>      options::NoArgumentUnused |
>      >>      options::CoreOption |
>      >>      options::LinkOption
>      >> ```
>      >>
>      >> ? However, there's a problem with this approach too: `clang
>     -help` will
>      >> stop working as the definition of `help` contains none of the above
>      >> flags [6]:
>      >>
>      >> ```
>      >> def help : Flag<["-", "--"], "help">, Flags<[CC1Option,CC1AsOption,
>      >> FC1Option, FlangOption]>,
>      >>    HelpText<"Display available options">;
>      >> ```
>      >>
>      >> This can be fixed by e.g. adding `CoreOption` to the definition
>     above.
>      >> But there will be more options with similar problem. There's also a
>      >> group of options that contain no flags at all (so it's hard to
>     classify
>      >> them). What should happen with them?
>      >>
>      >> # *LLVM's OPTTABLE*
>      >> Perhaps we should improve this in LLVM in OptTable.{h|cpp}
>     instead? For
>      >> instance, what's the relation between FlagsToExclude and
>     FlagsToInclude
>      >> [7]? Which one takes precedence? Note that any changes in LLVM's
>      >> OptTable will have impact beyond Clang and Flang, so I don't
>     want dive
>      >> too deep into this in this RFC. But I suspect that refining that
>     API a
>      >> bit could help Clang and Flang.
>
>     AFAIK the only users of OptTable are: clang (clang-cl), lld (lld-link,
>     ld.lld, wasm-ld), and llvm-symbolizer.
>     You can check whether their -help output changes with your refactoring.
>
>      >> # *SUGGESTION*
>      >> I think that some of these issues could be resolved (or at least
>      >> improved) if:
>      >>   * all options defined in clang's Options.td were required to
>     contain a
>      >> flag or otherwise be ignored
>      >>   * every flag clearly communicated the mode (e.g. compiler
>     driver vs
>      >> frontend driver) in which particular option is available (i.e. there
>      >> should be no need for `DriverOption` _and_ `NoDriverOption`)
>      >>   * every tool (e.g. `clang` or `clang -cc1`) was more explicit
>     about
>      >> the supported options (or, at least, options displayed when
>     passing `-help`)
>      >>
>      >> Do these suggestions make sense? Is my reasoning valid? Or did I
>     get it
>      >> completely wrong? :)
>      >>
>      >> Thanks for reading!
>      >> On behalf of the Arm Fortran team,
>      >> -Andrzej
>      >>
>      >> [1]
>      >>
>     https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.h#L26-L40
>      >> [2]
>      >>
>     https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/include/clang/Driver/Options.td#L19-L20
>      >> [3]
>      >>
>     https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5261-L5279
>      >> [4] https://reviews.llvm.org/D87989
>      >> [5]
>      >>
>     https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/clang/lib/Driver/Driver.cpp#L5263
>      >> [6]
>      >>
>     https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L2131-L2132
>      >> [7]
>      >>
>     https://github.com/llvm/llvm-project/blob/25692b7765e2364896a0136f5b54dde3de2dd563/llvm/include/llvm/Option/OptTable.h#L173-L175
>      >> _______________________________________________
>      >> cfe-dev mailing list
>      >> [hidden email] <mailto:[hidden email]>
>      >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>      >
>      > _______________________________________________
>      > cfe-dev mailing list
>      > [hidden email] <mailto:[hidden email]>
>      > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>     --
>     宋方睿
>
>
>
> --
> --Artem Belevich
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev