[PATCH] D124753: [HLSL] Set main as default entry.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D124753#3550670 <https://reviews.llvm.org/D124753#3550670>, @python3kgae wrote:

> In D124753#3550337 <https://reviews.llvm.org/D124753#3550337>, @Anastasia wrote:
>
>> In D124753#3546608 <https://reviews.llvm.org/D124753#3546608>, @python3kgae wrote:
>>
>>> In D124753#3545779 <https://reviews.llvm.org/D124753#3545779>, @Anastasia wrote:
>>>
>>>> From the current change it seems to me that what you need to be testing is a just that the frontend options are being passed correctly? This should then be a driver test with `-###` checking for the options to be set for the frontend invocation...
>>>
>>> There's already a driver test with '-###' in https://reviews.llvm.org/D124751#change-af6Z62NjlfGb
>>
>> This test doesn't seem to correspond to the change being added as you are changing the command-line flags. You don't actually add/generate any attributes in this patch.
>
> Sorry to make things confusing. I should not split default value for -E option as a separate PR :(
>
> There's dxc_E.hlsl (https://reviews.llvm.org/D124751#change-af6Z62NjlfGb) in https://reviews.llvm.org/D124751 where the -E option is added.
> dxc_E.hlsl will test -E option translated into -hlsl-entry for cc1.
>
> There was a test for codeGen of -E option in https://reviews.llvm.org/D124752, but I removed it because it is to almost the same as https://reviews.llvm.org/D124752#change-w4NWvaT68Dhk which test codeGen for ShaderAttr.
>
> And the entry_default.hlsl in current PR test codeGen for the default main and -E option.
>
> I can add a separate codeGen test for -E option if that's what you thought is missing.

Yes, in this patch you are just changing the marchaling flag to `-cc1` right? It would be better if you test just exactly that and then in dependent patches you can test what is related to other changes. It will make things less confusing then. ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124753