All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
To: Dibyendu Majumdar <mobile@majumdar.org.uk>
Cc: Linux-Sparse <linux-sparse@vger.kernel.org>,
	Christopher Li <sparse@chrisli.org>,
	Jeff Garzik <jeff@garzik.org>, Pekka Enberg <penberg@kernel.org>
Subject: Re: [PATCH v4 00/63] LLVM fixes
Date: Tue, 21 Mar 2017 15:55:08 +0100	[thread overview]
Message-ID: <20170321145507.x2bfcduvbrv25int@macbook.local> (raw)
In-Reply-To: <CACXZuxe8cg1QMnznpy8MCkZtsuG5mTd7zjZsapOupMk2jO+fAA@mail.gmail.com>

On Tue, Mar 21, 2017 at 01:36:50PM +0000, Dibyendu Majumdar wrote:
> Hi Luc,
> 
> On 21 March 2017 at 13:00, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > On Tue, Mar 21, 2017 at 11:24:57AM +0000, Dibyendu Majumdar wrote:
> >> Thank you once again for posting this set of patches. I wanted to
> >> understand what your approach is with testing of the LLVM backend in
> >> general. The test cases that are currently present and also the new
> >> ones you are adding do not appear to be runnable. Unless you are
> >> saving expected LLVM IR output and comparing the test output with that
> >> I think the tests do not prove that the generated code is correct.
> >
> > I'm not surprised at all that some of the test cases, new or old,
> > are not runnable (I suppose that you mean that their execution
> > produce wrong result).
> 
> I meant that they are not runnable as in you cannot run the compiled
> output. Moreover the tests do not validate the expected results.
> 
> >
> > It's a bit too early for me to look closely at the generated code,
> > there was simply too much input code that caused crashes, triggered
> > some asserts or produced type error in the LLVM IR. And it's not
> > like everything is now solved regarding this.
> >
> 
> Okay understand this.
> 
> > For the next steps, yes, it would certainly be needed to have tests
> > for the correctness of the generated code. And by 'tests' I mean
> > 'test cases suitable and present in sparse's test suite'.
> > I was somehow expecting you will submit them/somes as you seem
> > quite interested in sparse/LLVM.
> >
> 
> Indeed I am and I have a bunch of tests in my repository that I run
> everytime I change something. These tests are all runnable in the
> sense mentioned above, and all are designed to validate that results
> are as expected. I am happy to contribute these as I mentioned before
> - but I am not sure of the process. Do I just submit patches? Are
> there any specific needs for how the tests should be run?

I haven't yet really thought about it but here is a few things:
- small, specific tests are preferable but
- bigger tests have also their uses.
- testing the output of llvm-dis can be interesting but is very
  sensitive to details of the code (like names and such).
- 'sparsei' (sparse-llvm + lli) can be usefull for these tests
  (maybe you already use it?).

Otherwise, yes sending patches to the mailing list is the normal
way to contribute but maybe Chris is fine to accept other means too.
If needed I can be an intermeadiate in the process.

> I am very much interested in ensuring that as changes occur in sparse
> they do not break things. While I will catch these breaks in my
> repository it is better to catch them upstream.

It's very much the same for me, I much prefer having tests while
developing than going blindly, submitting a serie, waiting for
feedback and respin.
 
-- Luc

      reply	other threads:[~2017-03-21 15:04 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21  0:15 [PATCH v4 00/63] LLVM fixes Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 01/63] only output internal pointer value when verbose is set Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 02/63] allow binop simplification after canonicalization Luc Van Oostenryck
2017-03-24  5:00   ` Christopher Li
2017-03-24  9:43     ` Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 03/63] canonicalize compare instructions Luc Van Oostenryck
2017-03-24  5:12   ` Christopher Li
2017-03-24  8:11     ` Luc Van Oostenryck
2017-03-24 23:47       ` Christopher Li
2017-03-25  0:03         ` Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 04/63] add is_signed_type() Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 05/63] fix usage of inlined calls Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 06/63] inlined calls should not block BB packing Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 07/63] give function's arguments a type via OP_PUSH Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 08/63] give a type to OP_PHISOURCE Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 09/63] give a type to OP_SEL, always Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 10/63] give a type to OP_SWITCH Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 11/63] add doc about sparse's instructions/IR Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 12/63] add support for wider type in switch-case Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 13/63] llvm: remove unneeded arg 'module' Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 14/63] llvm: remove unneeded 'generation' Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 15/63] llvm: remove unneeded function::type Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 16/63] llvm: reduce scope of 'bb_nr' Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 17/63] llvm: use pseudo_list_size() instead of open coding it Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 18/63] llvm: give arguments a name Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 19/63] llvm: give a name to call's return values Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 20/63] llvm: avoid useless temp variable Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 21/63] llvm: extract get_sym_value() from pseudo_to_value() Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 22/63] llvm: fix test of floating-point type Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 23/63] llvm: fix translation of PSEUDO_VALs into a ValueRefs Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 24/63] llvm: fix output_op_store() which modify its operand Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 25/63] llvm: fix output_op_[ptr]cast() Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 26/63] llvm: take care of degenerated rvalues Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 27/63] llvm: add test cases for symbol's address Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 28/63] llvm: add test cases for pointers passed as argument Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 29/63] llvm: add test cases for arrays " Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 30/63] llvm: add test cases for degenerated pointers Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 31/63] llvm: add support for OP_NEG Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 32/63] llvm: add support for OP_SETVAL with floats Luc Van Oostenryck
2017-03-24  5:53   ` Christopher Li
2017-03-24  7:48     ` Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 33/63] llvm: add support for OP_SETVAL with labels Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 34/63] llvm: ignore OP_INLINED_CALL Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 35/63] llvm: fix pointer/float mixup in comparisons Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 36/63] llvm: fix type in comparison with an address constant Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 37/63] llvm: give correct type to binops Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 38/63] llvm: adjust OP_RET's type Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 39/63] llvm: variadic functions are not being marked as such Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 40/63] llvm: fix type of switch constants Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 41/63] llvm: make pseudo_name() more flexible Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 42/63] llvm: give a name to all values Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 43/63] llvm: add support for OP_SWITCH with a range Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 44/63] llvm: fix OP_SWITCH has no target Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 45/63] llvm: make value_to_pvalue() more flexible Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 46/63] llvm: make value_to_ivalue() " Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 47/63] llvm: add test case pointer compare with cast Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 48/63] llvm: let pseudo_to_value() directly use the type Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 49/63] llvm: remove unneeded pseudo_to_value() unneeded argument Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 50/63] llvm: introduce get_ioperand() Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 51/63] llvm: fix mutating function pointer Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 52/63] llvm: fix mutated OP_RET Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 53/63] llvm: fix mutated OP_SEL Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 54/63] llvm: fix mutated OP_SWITCH Luc Van Oostenryck
2017-03-21  0:15 ` [PATCH v4 55/63] llvm: fix mutated OP_PHISOURCE Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 56/63] llvm: fix mutated OP_[PTR]CAST Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 57/63] llvm: add support for restricted types Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 58/63] llvm: fix get value from initialized symbol Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 59/63] llvm: fix get value from non-anonymous symbol Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 60/63] llvm: fix type of bitfields Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 61/63] llvm: add support for OP_FPCAST Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 62/63] llvm: add support for cast from floats Luc Van Oostenryck
2017-03-21  0:16 ` [PATCH v4 63/63] llvm: cleanup of output_[ptr]cast() Luc Van Oostenryck
2017-03-21 10:29 ` [PATCH v4 00/63] LLVM fixes Pekka Enberg
2017-03-21 11:24 ` Dibyendu Majumdar
2017-03-21 13:00   ` Luc Van Oostenryck
2017-03-21 13:36     ` Dibyendu Majumdar
2017-03-21 14:55       ` Luc Van Oostenryck [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170321145507.x2bfcduvbrv25int@macbook.local \
    --to=luc.vanoostenryck@gmail.com \
    --cc=jeff@garzik.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=mobile@majumdar.org.uk \
    --cc=penberg@kernel.org \
    --cc=sparse@chrisli.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.