diff mbox series

Testsuite: Disable micromips for MSA tests

Message ID 20230221023857.211542-1-xin.liu@oss.cipunited.com
State Accepted
Headers
Series Testsuite: Disable micromips for MSA tests |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Xin Liu Feb. 21, 2023, 2:39 a.m. UTC 
  From: Matthew Fortune <matthew.fortune@imgtec.com>

---
 gcc/testsuite/gcc.target/mips/mips.exp | 1 +
 1 file changed, 1 insertion(+)

Comments

Jeff Law March 11, 2023, 3:42 p.m. UTC | #1 
On 2/20/23 19:39, Xin Liu wrote:
> From: Matthew Fortune <matthew.fortune@imgtec.com>
> 
> ---
>   gcc/testsuite/gcc.target/mips/mips.exp | 1 +
>   1 file changed, 1 insertion(+)
Unfortunately, you haven't given anyone any background that would allow 
them to evaluate/review this patch.

I'm guessing that MSA does not support micromips, is that correct?

And for the future, please include a ChangeLog entry with patches.  A 
ChangeLog entry describes what changed, not why something changes.  A 
reasonable ChangeLog for this patch might be:

gcc/testsuite:
	* gcc.target/mips/mips.exp (mips-dg-options): Disable micromips
	for MSA tests.

Jeff
Xin Liu March 14, 2023, 5:46 a.m. UTC | #2 
Thanks for your feedback. You're right that MicroMIPS doesn't support 
MSA, so disabling micromips for MSA tests is a reasonable change.
I'll make sure to include a ChangeLog entry with a clear description of 
future patches. Thanks for the suggestions, and I'll strive to improve 
my work based on your feedback.

On 2023/3/11 23:42, Jeff Law wrote:
>
>
> On 2/20/23 19:39, Xin Liu wrote:
>> From: Matthew Fortune <matthew.fortune@imgtec.com>
>>
>> ---
>>   gcc/testsuite/gcc.target/mips/mips.exp | 1 +
>>   1 file changed, 1 insertion(+)
> Unfortunately, you haven't given anyone any background that would 
> allow them to evaluate/review this patch.
>
> I'm guessing that MSA does not support micromips, is that correct?
>
> And for the future, please include a ChangeLog entry with patches.  A 
> ChangeLog entry describes what changed, not why something changes.  A 
> reasonable ChangeLog for this patch might be:
>
> gcc/testsuite:
>     * gcc.target/mips/mips.exp (mips-dg-options): Disable micromips
>     for MSA tests.
>
> Jeff
Jeff Law March 19, 2023, 5:31 p.m. UTC | #3 
On 3/13/23 23:46, Xin Liu wrote:
> Thanks for your feedback. You're right that MicroMIPS doesn't support
> MSA, so disabling micromips for MSA tests is a reasonable change.
> I'll make sure to include a ChangeLog entry with a clear description of
> future patches. Thanks for the suggestions, and I'll strive to improve
> my work based on your feedback.
THanks.  I've pushed your patch to the trunk.

jeff
>
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.target/mips/mips.exp b/gcc/testsuite/gcc.target/mips/mips.exp
index 81e19f39853..bf32fe0c93f 100644
--- a/gcc/testsuite/gcc.target/mips/mips.exp
+++ b/gcc/testsuite/gcc.target/mips/mips.exp
@@ -1463,6 +1463,7 @@  proc mips-dg-options { args } {
     mips_option_dependency options "-msoft-float" "-mno-paired-single"
     mips_option_dependency options "-mno-paired-single" "-mno-mips3d"
+    mips_option_dependency options "-mmsa" "-mno-micromips"
     mips_option_dependency options "-mmsa" "-mno-mips16"
 
     # If the test requires an unsupported option, change run tests
     # to link tests.