Hi Eric, Could you explain the intent and policy regarding the test-suite body of code. Should the test be left as much as possible as-is (even if technically incorrect)? Should changes only affect the XCore target (#ifdef) or should all targets get the changes? Taking "int32_t main" as an example. The correct return type & argc for main is 'int'. In the XCore tool chain, 'int32_t' equates to long (IIRC) and hence is not acceptable in the type signature for main. Should this change be only for the XCore target or all targets? When I know the policy for the test-suite, I'll alter as necessary & regroup the changes into patches containing the same type of change and submit for approval. One more question: On patch I need to address is how to make deterministic the order of stdout & stderr. Ideally, applications would use either stdout or stderr but not both. Would a patch to change to only stdout be acceptable (plus any changes to expected output)? Thank you Robert ________________________________ From: Eric Christopher [echristo at gmail.com] Sent: 13 January 2014 19:16 To: Robert Lytton; llvmdev at cs.uiuc.edu Subject: [LLVMdev] test suite 'owner' Some of these are pretty weird, e.g. int32_t main. Probably the best thing is to submit each patch individually with an explanation of what the purpose is and we can talk about them then. -eric On Fri Jan 10 2014 at 4:13:47 AM, Robert Lytton <robert at xmos.com<mailto:robert at xmos.com>> wrote: Hi, I have found it necessary to make some changes to the test-suite for the XCore platform. These changes include: altering #includes, as supported by XCore; using stdout or stderr to make the output diffs consistent (fixing expected output too); (This work is still under review as best way to do it) 'fixing' symbol and type problems e.g name clashes & scope; altering/adding code snippets and macros. I have used #ifdef to limit and keep any changes specific to the XCore. Some of these could/should be made common to all targets e.g. log2() -> logTwo(). I have also altered the Makefile to filter out tests not supported by the XCore. I would like to discuss the changes I have found necessary to make and what is the next step. Should any/all of them be pushed upstream? Robert -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140113/980a9142/attachment.html>
The idea is that it's going to be a correct and portable set of code that works both as a correctness and performance test suite. -eric On Mon Jan 13 2014 at 12:22:51 PM, Robert Lytton <robert at xmos.com> wrote:> Hi Eric, > > Could you explain the intent and policy regarding the test-suite body of > code. > Should the test be left as much as possible as-is (even if technically > incorrect)? > Should changes only affect the XCore target (#ifdef) or should all targets > get the changes? > > Taking "int32_t main" as an example. > The correct return type & argc for main is 'int'. > In the XCore tool chain, 'int32_t' equates to long (IIRC) and hence is not > acceptable in the type signature for main. > Should this change be only for the XCore target or all targets? > > When I know the policy for the test-suite, I'll alter as necessary & > regroup the changes into patches containing the same type of change and > submit for approval. > > One more question: > On patch I need to address is how to make deterministic the order of > stdout & stderr. > Ideally, applications would use either stdout or stderr but not both. > Would a patch to change to only stdout be acceptable (plus any changes to > expected output)? > > Thank you > > Robert > > > > ------------------------------ > *From:* Eric Christopher [echristo at gmail.com] > *Sent:* 13 January 2014 19:16 > *To:* Robert Lytton; llvmdev at cs.uiuc.edu > *Subject:* [LLVMdev] test suite 'owner' > > Some of these are pretty weird, e.g. int32_t main. Probably the best > thing is to submit each patch individually with an explanation of what the > purpose is and we can talk about them then. > > -eric > > > On Fri Jan 10 2014 at 4:13:47 AM, Robert Lytton <robert at xmos.com> wrote: > > Hi, > > I have found it necessary to make some changes to the test-suite for the > XCore platform. > > These changes include: > altering #includes, as supported by XCore; > using stdout or stderr to make the output diffs consistent (fixing > expected output too); > (This work is still under review as best way to do it) > 'fixing' symbol and type problems e.g name clashes & scope; > altering/adding code snippets and macros. > > I have used #ifdef to limit and keep any changes specific to the XCore. > Some of these could/should be made common to all targets e.g. log2() -> > logTwo(). > > I have also altered the Makefile to filter out tests not supported by the > XCore. > > I would like to discuss the changes I have found necessary to make and > what is the next step. > Should any/all of them be pushed upstream? > > > Robert > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140113/9318e28f/attachment.html>
... and so (I infer from that) it should not be patched let alone need any changes. Assuming my inference is correct, any patching should only affect the XCore target and only if there is a good reason why the XCore requires the change. So, is #ifdef around all/most changes the correct way to submit a patch? Robert ________________________________ From: Eric Christopher [echristo at gmail.com] Sent: 13 January 2014 20:24 To: Robert Lytton; llvmdev at cs.uiuc.edu Subject: RE: [LLVMdev] test suite 'owner' The idea is that it's going to be a correct and portable set of code that works both as a correctness and performance test suite. -eric On Mon Jan 13 2014 at 12:22:51 PM, Robert Lytton <robert at xmos.com<mailto:robert at xmos.com>> wrote: Hi Eric, Could you explain the intent and policy regarding the test-suite body of code. Should the test be left as much as possible as-is (even if technically incorrect)? Should changes only affect the XCore target (#ifdef) or should all targets get the changes? Taking "int32_t main" as an example. The correct return type & argc for main is 'int'. In the XCore tool chain, 'int32_t' equates to long (IIRC) and hence is not acceptable in the type signature for main. Should this change be only for the XCore target or all targets? When I know the policy for the test-suite, I'll alter as necessary & regroup the changes into patches containing the same type of change and submit for approval. One more question: On patch I need to address is how to make deterministic the order of stdout & stderr. Ideally, applications would use either stdout or stderr but not both. Would a patch to change to only stdout be acceptable (plus any changes to expected output)? Thank you Robert ________________________________ From: Eric Christopher [echristo at gmail.com<mailto:echristo at gmail.com>] Sent: 13 January 2014 19:16 To: Robert Lytton; llvmdev at cs.uiuc.edu<mailto:llvmdev at cs.uiuc.edu> Subject: [LLVMdev] test suite 'owner' Some of these are pretty weird, e.g. int32_t main. Probably the best thing is to submit each patch individually with an explanation of what the purpose is and we can talk about them then. -eric On Fri Jan 10 2014 at 4:13:47 AM, Robert Lytton <robert at xmos.com<mailto:robert at xmos.com>> wrote: Hi, I have found it necessary to make some changes to the test-suite for the XCore platform. These changes include: altering #includes, as supported by XCore; using stdout or stderr to make the output diffs consistent (fixing expected output too); (This work is still under review as best way to do it) 'fixing' symbol and type problems e.g name clashes & scope; altering/adding code snippets and macros. I have used #ifdef to limit and keep any changes specific to the XCore. Some of these could/should be made common to all targets e.g. log2() -> logTwo(). I have also altered the Makefile to filter out tests not supported by the XCore. I would like to discuss the changes I have found necessary to make and what is the next step. Should any/all of them be pushed upstream? Robert -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140113/f4601ad3/attachment.html>
On Mon Jan 13 2014 at 12:25:14 PM, Robert Lytton <robert at xmos.com> wrote:> Hi Eric, > > Could you explain the intent and policy regarding the test-suite body of > code. > Should the test be left as much as possible as-is (even if technically > incorrect)? > Should changes only affect the XCore target (#ifdef) or should all targets > get the changes? > > Taking "int32_t main" as an example. > The correct return type & argc for main is 'int'. > In the XCore tool chain, 'int32_t' equates to long (IIRC) and hence is not > acceptable in the type signature for main. > Should this change be only for the XCore target or all targets? >All targets. This code is incorrect/non-portable. The return type of main is 'int' per the language spec.> > When I know the policy for the test-suite, I'll alter as necessary & > regroup the changes into patches containing the same type of change and > submit for approval. > > One more question: > On patch I need to address is how to make deterministic the order of > stdout & stderr. > Ideally, applications would use either stdout or stderr but not both. > Would a patch to change to only stdout be acceptable (plus any changes to > expected output)? > > Thank you > > Robert > > > > ------------------------------ > *From:* Eric Christopher [echristo at gmail.com] > *Sent:* 13 January 2014 19:16 > *To:* Robert Lytton; llvmdev at cs.uiuc.edu > *Subject:* [LLVMdev] test suite 'owner' > > Some of these are pretty weird, e.g. int32_t main. Probably the best > thing is to submit each patch individually with an explanation of what the > purpose is and we can talk about them then. > > -eric > > > On Fri Jan 10 2014 at 4:13:47 AM, Robert Lytton <robert at xmos.com> wrote: > > Hi, > > I have found it necessary to make some changes to the test-suite for the > XCore platform. > > These changes include: > altering #includes, as supported by XCore; > using stdout or stderr to make the output diffs consistent (fixing > expected output too); > (This work is still under review as best way to do it) > 'fixing' symbol and type problems e.g name clashes & scope; > altering/adding code snippets and macros. > > I have used #ifdef to limit and keep any changes specific to the XCore. > Some of these could/should be made common to all targets e.g. log2() -> > logTwo(). > > I have also altered the Makefile to filter out tests not supported by the > XCore. > > I would like to discuss the changes I have found necessary to make and > what is the next step. > Should any/all of them be pushed upstream? > > > Robert > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140114/edd75d96/attachment.html>
thank you. I'll submit the patch without #ifdef in this case. Robert ________________________________ From: dblaikie at gmail.com [dblaikie at gmail.com] Sent: 14 January 2014 17:03 To: Robert Lytton; echristo at gmail.com; llvmdev at cs.uiuc.edu Subject: Re: [LLVMdev] test suite 'owner' On Mon Jan 13 2014 at 12:25:14 PM, Robert Lytton <robert at xmos.com<mailto:robert at xmos.com>> wrote: Hi Eric, Could you explain the intent and policy regarding the test-suite body of code. Should the test be left as much as possible as-is (even if technically incorrect)? Should changes only affect the XCore target (#ifdef) or should all targets get the changes? Taking "int32_t main" as an example. The correct return type & argc for main is 'int'. In the XCore tool chain, 'int32_t' equates to long (IIRC) and hence is not acceptable in the type signature for main. Should this change be only for the XCore target or all targets? All targets. This code is incorrect/non-portable. The return type of main is 'int' per the language spec. When I know the policy for the test-suite, I'll alter as necessary & regroup the changes into patches containing the same type of change and submit for approval. One more question: On patch I need to address is how to make deterministic the order of stdout & stderr. Ideally, applications would use either stdout or stderr but not both. Would a patch to change to only stdout be acceptable (plus any changes to expected output)? Thank you Robert ________________________________ From: Eric Christopher [echristo at gmail.com<mailto:echristo at gmail.com>] Sent: 13 January 2014 19:16 To: Robert Lytton; llvmdev at cs.uiuc.edu<mailto:llvmdev at cs.uiuc.edu> Subject: [LLVMdev] test suite 'owner' Some of these are pretty weird, e.g. int32_t main. Probably the best thing is to submit each patch individually with an explanation of what the purpose is and we can talk about them then. -eric On Fri Jan 10 2014 at 4:13:47 AM, Robert Lytton <robert at xmos.com<mailto:robert at xmos.com>> wrote: Hi, I have found it necessary to make some changes to the test-suite for the XCore platform. These changes include: altering #includes, as supported by XCore; using stdout or stderr to make the output diffs consistent (fixing expected output too); (This work is still under review as best way to do it) 'fixing' symbol and type problems e.g name clashes & scope; altering/adding code snippets and macros. I have used #ifdef to limit and keep any changes specific to the XCore. Some of these could/should be made common to all targets e.g. log2() -> logTwo(). I have also altered the Makefile to filter out tests not supported by the XCore. I would like to discuss the changes I have found necessary to make and what is the next step. Should any/all of them be pushed upstream? Robert -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140115/4b15c997/attachment.html>