Re: committed [RISC-V]: Harden test scan patterns

[Jeff Law <jeffreyalaw@gmail.com>]

On 9/27/23 17:21, Vineet Gupta wrote:
> 
> 
> On 9/27/23 13:14, Jeff Law wrote:
>>>> It would help to describe how these patterns were under specified so
>>>> that folks don't continue to make the same mistake as new tests get 
>>>> added.
>>>
>>> dg-final scan-assembler, scan-assembler-not, and scan-assembler-times
>>> use a tcl regular expression (often referred to abbreviated as RE), as
>>> described in https://www.tcl.tk/man/tcl8.4/TclCmd/re_syntax.html .
>>>
>>> If your RE is not specific enough, it can match LTO information that the
>>> compiler places into its assembly output when the relevant options are
>>> provided, which is common when running tests where the test harness
>>> iterates over a number of optimization option combinations.
>>> Note that '.' is an atom that can match any character.  If you want to
>>> match a dot specifically, you have to escape it with a backslash: '\.' .
>>> When you are matching an instruction mnemonic, an effective way to
>>> avoid matching in LTO information is to enforce matching of word start
>>> (\m) and/or word end (\M) .
>>> Note also that the backslash has to be quoted.  If the RE is enclosed in
>>> '"' quotes, extra backslashes are needed.  That is not necessary when it
>>> is enclosed in curly braces.
>>>
>>> For example, "ld.w" will be matched in:
>>>
>>> .ascii "h\227\022\212ld@w\251jr\254'\320\255vwj\252\026\016\364"
>>>
>>> If you write {\mld\.w\M} instead, you avoid this problem.
>> OK.  So that naturally leads to the question, why aren't others seeing 
>> this, both in the RISC-V world and more generally.  I'm not aware of 
>> any case where I've run the testsuite and tripped over this issue, nor 
>> am I aware of anyone else tripping over it.
> 
> Actually I did run into it. See commit ecfa870ff29d979bd2c ("RISC-V: 
> optim const DF +0.0 store to mem [PR/110748]") where a false failure was 
> triggered due to these random LTO strings and needed adjusting.
Ah!  Good (I suppose).

> 
> -/* { dg-final { scan-assembler-not "sw" } } */
> -/* { dg-final { scan-assembler-not "fld" } } */
> -/* { dg-final { scan-assembler-not "fsd" } } */
> -/* { dg-final { scan-assembler-not "lw" } } */
> +/* { dg-final { scan-assembler-not "\tsw\t" } } */
> +/* { dg-final { scan-assembler-not "\tfld\t" } } */
> +/* { dg-final { scan-assembler-not "\tfsd\t" } } */
> +/* { dg-final { scan-assembler-not "\tlw\t" } } */
The downside of this approach (which I nearly suggested to Joern) is 
that it relies on tabs before/after the mnemonic.  That would also imply 
a review policy to ensure we always use tabs -- and there's going to be 
cases where spotting a tab vs space is going to be tough in a patch 
review, particularly after the mnemonic.

We could instead match a regexp that allows spaces or tabs, but at that 
point I doubt it's simpler than Joern's approach.

So I recommend we go forward with Joern's approach (so consider that an 
ACK for the trunk).   Joern  can you post a follow-up manual twiddle so 
that other ports can follow your example and avoid this problem?

THanks,

jeff