Sorry, I was thinking to review the test but didn't. Is this test complete? It does invoke lld, but it didn't verify its output. On Mon, Jul 30, 2018 at 2:03 PM Andrew Kelley <superjoe30 at gmail.com> wrote:> Ping Rui. Is there anything else that needs to be done on this patch? > > On Tue, Jul 17, 2018 at 6:58 AM, Carlo Kok via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Got it. >> >> Attached are both the testcase & the fix. >> >> On Tue, Jul 17, 2018, at 12:06, Carlo Kok via llvm-dev wrote: >> > On Wed, Jul 11, 2018, at 16:45, Davide Italiano wrote: >> > > On Tue, Jul 10, 2018 at 10:12 PM Carlo Kok via llvm-dev >> > > <llvm-dev at lists.llvm.org> wrote: >> > > > >> > > > That sounds quite reasaonable; how does one usually go about doing >> that? a repro zip that hits both asserts? >> > > > >> > > >> > > You can take inspiration from anything in lld/test, but basically >> > > either an assembly source (or multiple) passed through llvm-mc and >> > > then lld, or a YAML file passed to yaml2obj (and then lld). From what >> > > I see the Mach-O backend prefers YAML-style testing, but I don't think >> > > that's a strict requirement. >> > >> > Doesn't look like the test system works on windows? >> > >> > >> > C:\p\llvm\llvm\tools\lld\test>python c:\p\llvm\llvm-bin32\relwithdebinfo >> > \bin\llvm-lit.py darwin\darwin-asserts-in-x86_64.ll >> > llvm-lit.py: C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py:101: fatal: >> > unable to parse config file 'C:\\p\\llvm\\llvm\\tools\\lld\\test\ >> > \lit.cfg.py', traceback: Traceback (most recent call last): >> > File "C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py", line 88, in >> > load_from_path >> > exec(compile(data, path, 'exec'), cfg_globals, None) >> > File "C:\p\llvm\llvm\tools\lld\test\lit.cfg.py", line 23, in <module> >> > config.test_format = lit.formats.ShTest(not >> > llvm_config.use_lit_shell) >> > AttributeError: 'NoneType' object has no attribute 'use_lit_shell' >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > llvm-dev at lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > Email had 1 attachment: >> > + darwin-asserts-in-x86_64.ll >> > 5k (application/octet-stream) >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180730/776ec526/attachment.html>
When I wrote the test, it failed lld (assertion) in debug mode. I can't get it to fail for non debug but afaik the test system should fail on lld asserts? On Mon, Jul 30, 2018, at 23:06, Rui Ueyama wrote:> Sorry, I was thinking to review the test but didn't. > > Is this test complete? It does invoke lld, but it didn't verify > its output.> > On Mon, Jul 30, 2018 at 2:03 PM Andrew Kelley > <superjoe30 at gmail.com> wrote:>> Ping Rui. Is there anything else that needs to be done on this patch?>> >> On Tue, Jul 17, 2018 at 6:58 AM, Carlo Kok via llvm-dev <llvm- >> dev at lists.llvm.org> wrote:>>> Got it. >>> >>> Attached are both the testcase & the fix. >>> >>> >>> On Tue, Jul 17, 2018, at 12:06, Carlo Kok via llvm-dev wrote: >>> > On Wed, Jul 11, 2018, at 16:45, Davide Italiano wrote: >>> > > On Tue, Jul 10, 2018 at 10:12 PM Carlo Kok via llvm-dev >>> > > <llvm-dev at lists.llvm.org> wrote: >>> > > > >>> > > > That sounds quite reasaonable; how does one usually go about >>> > > > doing that? a repro zip that hits both asserts?>>> > > > >>> > > >>> > > You can take inspiration from anything in lld/test, but >>> > > basically>>> > > either an assembly source (or multiple) passed through llvm-mc >>> > > and>>> > > then lld, or a YAML file passed to yaml2obj (and then lld). From >>> > > what>>> > > I see the Mach-O backend prefers YAML-style testing, but I don't >>> > > think>>> > > that's a strict requirement. >>> > >>> > Doesn't look like the test system works on windows? >>> > >>> > >>> > C:\p\llvm\llvm\tools\lld\test>python c:\p\llvm\llvm- >>> > bin32\relwithdebinfo>>> > \bin\llvm-lit.py darwin\darwin-asserts-in-x86_64.ll >>> > llvm-lit.py: C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py:101: >>> > fatal:>>> > unable to parse config file 'C:\\p\\llvm\\llvm\\tools\\lld\\test\>>> > \lit.cfg.py', traceback: Traceback (most recent call last): >>> > File "C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py", line 88, >>> > in>>> > load_from_path >>> > exec(compile(data, path, 'exec'), cfg_globals, None) >>> > File "C:\p\llvm\llvm\tools\lld\test\lit.cfg.py", line 23, in >>> > <module>>>> > config.test_format = lit.formats.ShTest(not >>> > llvm_config.use_lit_shell) >>> > AttributeError: 'NoneType' object has no attribute 'use_lit_shell'>>> > >>> > _______________________________________________ >>> > LLVM Developers mailing list llvm-dev at lists.llvm.org >>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> > Email had 1 attachment:>>> > + darwin-asserts-in-x86_64.ll >>> > 5k (application/octet-stream) >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180731/86933415/attachment.html>
Will this patch make it into LLD 7.0.0 release? It's currently the only patch that zig's LLD fork has on top of upstream. On Tue, Jul 31, 2018 at 12:49 AM Carlo Kok <ck at remobjects.com> wrote:> When I wrote the test, it failed lld (assertion) in debug mode. I can't > get it to fail for non debug but afaik the test system should fail on lld > asserts? > > > On Mon, Jul 30, 2018, at 23:06, Rui Ueyama wrote: > > Sorry, I was thinking to review the test but didn't. > > Is this test complete? It does invoke lld, but it didn't verify its output. > > On Mon, Jul 30, 2018 at 2:03 PM Andrew Kelley <superjoe30 at gmail.com> > wrote: > > Ping Rui. Is there anything else that needs to be done on this patch? > > On Tue, Jul 17, 2018 at 6:58 AM, Carlo Kok via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > Got it. > > Attached are both the testcase & the fix. > > > On Tue, Jul 17, 2018, at 12:06, Carlo Kok via llvm-dev wrote: > > On Wed, Jul 11, 2018, at 16:45, Davide Italiano wrote: > > > On Tue, Jul 10, 2018 at 10:12 PM Carlo Kok via llvm-dev > > > <llvm-dev at lists.llvm.org> wrote: > > > > > > > > That sounds quite reasaonable; how does one usually go about doing > that? a repro zip that hits both asserts? > > > > > > > > > > You can take inspiration from anything in lld/test, but basically > > > either an assembly source (or multiple) passed through llvm-mc and > > > then lld, or a YAML file passed to yaml2obj (and then lld). From what > > > I see the Mach-O backend prefers YAML-style testing, but I don't think > > > that's a strict requirement. > > > > Doesn't look like the test system works on windows? > > > > > > C:\p\llvm\llvm\tools\lld\test>python c:\p\llvm\llvm-bin32\relwithdebinfo > > \bin\llvm-lit.py darwin\darwin-asserts-in-x86_64.ll > > llvm-lit.py: C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py:101: fatal: > > unable to parse config file 'C:\\p\\llvm\\llvm\\tools\\lld\\test\ > > \lit.cfg.py', traceback: Traceback (most recent call last): > > File "C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py", line 88, in > > load_from_path > > exec(compile(data, path, 'exec'), cfg_globals, None) > > File "C:\p\llvm\llvm\tools\lld\test\lit.cfg.py", line 23, in <module> > > config.test_format = lit.formats.ShTest(not > > llvm_config.use_lit_shell) > > AttributeError: 'NoneType' object has no attribute 'use_lit_shell' > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > Email had 1 attachment: > > + darwin-asserts-in-x86_64.ll > > 5k (application/octet-stream) > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180804/64b01978/attachment.html>
I'd suggest adding a comment to the test explaining what it's checking for: "; Check that lld doesn't assert when bla bla...". Checking for the expected output is nicer, but sometimes that's hard. In the test run lines, you should replace "darwin-asserts-in-x86_64.ll" with %s and darwin-asserts-in-x86_64.o with %t.o and the lit will expand those to the source file and to a temporary name, respectively. Rui, do you have any other concerns? On Tue, Jul 31, 2018 at 6:49 AM, Carlo Kok via llvm-dev <llvm-dev at lists.llvm.org> wrote:> When I wrote the test, it failed lld (assertion) in debug mode. I can't get > it to fail for non debug but afaik the test system should fail on lld > asserts? > > > On Mon, Jul 30, 2018, at 23:06, Rui Ueyama wrote: > > Sorry, I was thinking to review the test but didn't. > > Is this test complete? It does invoke lld, but it didn't verify its output. > > On Mon, Jul 30, 2018 at 2:03 PM Andrew Kelley <superjoe30 at gmail.com> wrote: > > Ping Rui. Is there anything else that needs to be done on this patch? > > On Tue, Jul 17, 2018 at 6:58 AM, Carlo Kok via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > Got it. > > Attached are both the testcase & the fix. > > > On Tue, Jul 17, 2018, at 12:06, Carlo Kok via llvm-dev wrote: >> On Wed, Jul 11, 2018, at 16:45, Davide Italiano wrote: >> > On Tue, Jul 10, 2018 at 10:12 PM Carlo Kok via llvm-dev >> > <llvm-dev at lists.llvm.org> wrote: >> > > >> > > That sounds quite reasaonable; how does one usually go about doing >> > > that? a repro zip that hits both asserts? >> > > >> > >> > You can take inspiration from anything in lld/test, but basically >> > either an assembly source (or multiple) passed through llvm-mc and >> > then lld, or a YAML file passed to yaml2obj (and then lld). From what >> > I see the Mach-O backend prefers YAML-style testing, but I don't think >> > that's a strict requirement. >> >> Doesn't look like the test system works on windows? >> >> >> C:\p\llvm\llvm\tools\lld\test>python c:\p\llvm\llvm-bin32\relwithdebinfo >> \bin\llvm-lit.py darwin\darwin-asserts-in-x86_64.ll >> llvm-lit.py: C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py:101: fatal: >> unable to parse config file 'C:\\p\\llvm\\llvm\\tools\\lld\\test\ >> \lit.cfg.py', traceback: Traceback (most recent call last): >> File "C:/p/llvm/llvm\utils\lit\lit\TestingConfig.py", line 88, in >> load_from_path >> exec(compile(data, path, 'exec'), cfg_globals, None) >> File "C:\p\llvm\llvm\tools\lld\test\lit.cfg.py", line 23, in <module> >> config.test_format = lit.formats.ShTest(not >> llvm_config.use_lit_shell) >> AttributeError: 'NoneType' object has no attribute 'use_lit_shell' >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> Email had 1 attachment: >> + darwin-asserts-in-x86_64.ll >> 5k (application/octet-stream) > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >