[PATCH] D85414: [test][DebugInfo] Adapt two tests for Sun assembler syntax on Sparc

Rainer Orth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 6 02:52:17 PDT 2020


ro added a comment.

FWIW, I've now posted the alternative patch to switch Sparc to the common ELF `.section` syntax instead: D85415 <https://reviews.llvm.org/D85415>.  If we go this way, the current patch becomes moot.



================
Comment at: llvm/test/DebugInfo/Generic/2010-06-29-InlinedFnLocalVar.ll:4
 
-; CHECK: debug_info,
+; CHECK: debug_info{{,|$}}
 ; CHECK:	DW_TAG_structure_type
----------------
jhenderson wrote:
> Unless you think it's important to verify the name exactly ends with "debug_info" (note that this line doesn't check that the name starts with "debug_info", so could pass with e.g. ".some_random_text_debug_info"), you can just get rid of the ',' instead.
> 
> Honestly, I'm not really sure what the point of checking the line is at all... It doesn't prove that the next two lines are in the .debug_info section, because a) the first check doesn't actually check the full section directive to show that it's referring to a section directive, and b) there could be anything, including other section directives between the check and the lines below... Same goes with the other test. I'm not sure if it's worth fixing things further thought...
> Unless you think it's important to verify the name exactly ends with "debug_info" (note that this line doesn't check that the name starts with "debug_info", so could pass with e.g. ".some_random_text_debug_info"), you can just get rid of the ',' instead.

Removing the `,` was my first thought.  However, there are a couple of lines matching just `debug_info`:
```
	.section	.debug_info
	.word	.Ldebug_info_end0-.Ldebug_info_start0 ! Length of Unit
.Ldebug_info_start0:
.Ldebug_info_end0:
```

> Honestly, I'm not really sure what the point of checking the line is at all... It doesn't prove that the next two lines are in the .debug_info section, because a) the first check doesn't actually check the full section directive to show that it's referring to a section directive, and b) there could be anything, including other section directives between the check and the lines below... Same goes with the other test. I'm not sure if it's worth fixing things further thought...

I'd been wary of tightening the check because I'm not sure which systems with non-ELF assembler syntaxes might be affected (e.g. Darwin), so this felt like a can of worms to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85414/new/

https://reviews.llvm.org/D85414



More information about the llvm-commits mailing list