[llvm-commits] [llvm] r157015 - in /llvm/trunk: ./ autoconf/ test/ test/ExecutionEngine/MCJIT/

[Danil Malyshev]

Hi Eli,

> Regarding (1), I guess this is a philosophical discussion of XFAIL vs. UNSUPPORTED, which is
> appropriate when. In this case (of MCJIT), if we have tests that are currently failing but we generally
> expect them to be fixed and pass at some point in time, I don't see why XFAIL is inappropriate.

In fact, there is no philosophy.
UNSUPPORTED test just mean that this test for this build unsupported. The execution result of this test can be various, it's can be success in a current SVN revision and fail in the next revision. It makes no sense to execute this test before the functionality is ready for selected build.
If the test marked as XFAIL, we clearly envision that the result of the test is always fail and we known a reason of the fail. And in this case the tested functionality can be 100% worked, just the test checks limits, for example.
Of course, it's ideal and not all tests adhere to these rules, but it is good to strive for this. And is good when test environment allows to strive for this.

> Thanks for the insights. Still, I think that expanding the infrastructure to support additional use cases is
> preferable to duplicating a ton of tests (I expect the ExecutionEngine test suite to grow considerably
> when MCJIT replaces JIT for more and more uses).

Of course, you are right. But the functional of the duplicating a ton of test should be covered by the additional infrastructure before it's can be merged, isn't it?

> I'm attaching the latest patch I have that adds SUBTEST to lit (with some examples, so it should be
> fairly obvious how it works). If you could also take a look, and suggest how the special case(s) you
> mention can be fitted into this scheme, that would be great.

Thanks for the patch, I looked at it. At first, I wrote the requirements for MCJIT testing, and then I wrote my thoughts, as this patch may be improved.
Any MCJIT test should be executed under the following conditions (the XFAIL can be used instead UNSUPPORTED at a first approximation, although it is better to do it right):
1. Host platform should be supported by MCJIT
2. Host architecture should be supported by MCJIT (it's would be nice a separate the x86 and the x86_64 here, because, for example, all MCJIT tests works fine on the 64bit darwin, but some of the tests fails on the 32bit darwin).
3. The 'targets' should contain the host architecture (because cross build may be configured witch only one target arch and the tests fails even if the host platform/arch will be supported by MCJIT).

These conditions can be checked in the lit.local.cfg right now.
So, I think the best option is make in the lit.local.cfg a list of named groups with marks is this group should be executed, or not. And check the cur_subtest_name with these list.
For example, we can use in each of ExecutionEngine tests these subtest:
; SUBTEST: JIT
and
; SUBTEST: MCJIT
Use the separated filters for groups JIT and MCJIT in the lit.local.cfg and mark each of the groups is this group supported, or not (it's can be more improved in future to more flags: success, unsupported, fail, maybe some else, but it no necessary right now).
Another way is improve the XFAIL filter, add to this filter possibilities checks the host arch, platform, targets. I think this way is easier for implements than the first, but its unattractive and heavy to use, because the will be need add the conditions to each of the tests and each of the subtest.


Regards,
Danil


________________________________________
From: Bendersky, Eli [eli.bendersky at intel.com]
Sent: Monday, May 21, 2012 11:04 PM
To: Danil Malyshev; Jim Grosbach; Daniel Dunbar (daniel.dunbar at gmail.com)
Cc: llvm-commits at cs.uiuc.edu for LLVM
Subject: RE: [llvm-commits] [llvm] r157015 - in /llvm/trunk: ./ autoconf/ test/ test/ExecutionEngine/MCJIT/

> Hi Eli,
>
> The idea of ​​using SUBTEST are interesting.
> However, I want to voice some of the problems, because I do not know
> whether you discussed it with Daniel and if you have a solution for them.
> 1. Currently we can use the lit.local.cfg to choose the tests which should be
> executed for current platform, or should be skipped. If test should be
> skipped this test will be marked as UNSUPPORTED.
> Inside the test, we can only choose whether we expect from the test that it
> will fails, or that is executed without error, the test will be executed in any
> case.
> I think if some functionality should not work for current build, the tests for
> this functionality should not be executed at all and should be marked as
> UNSUPPORTED. This is a more valid option than "expected fail". Accordingly,
> it is necessary to move this functionality into the test to be able to use it for
> SUBTEST.
> 2. At present, the flag XFAIL checks the value of target_triple. That's not
> enough for the tests MCJIT, as, for example, among buildboots we have a
> cross-builds, which Windows host, and ARM target. In this case, the host
> platform is not supported by MCJIT, but the target platform is supported. So
> the MCJIT tests will be executed, will fails and will be marked as "unexpected
> fails".
>
> Regards,
> Danil

Hi Danil,

Thanks for the insights. Still, I think that expanding the infrastructure to support additional use cases is preferable to duplicating a ton of tests (I expect the ExecutionEngine test suite to grow considerably when MCJIT replaces JIT for more and more uses).

Regarding (1), I guess this is a philosophical discussion of XFAIL vs. UNSUPPORTED, which is appropriate when. In this case (of MCJIT), if we have tests that are currently failing but we generally expect them to be fixed and pass at some point in time, I don't see why XFAIL is inappropriate.

Regarding (2) - I must admit I didn't study the issue of cross builds, but here agai, the infrastructure (i.e. lit) can be enhanced to support this use case.

I'm attaching the latest patch I have that adds SUBTEST to lit (with some examples, so it should be fairly obvious how it works). If you could also take a look, and suggest how the special case(s) you mention can be fitted into this scheme, that would be great.

Eli







---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.