[PATCH] D126361: [DebugInfo][ARM] Not readonly check for RWPI globals

[Keith Walker via Phabricator via llvm-commits]

keith.walker.arm added inline comments.


================
Comment at: llvm/test/DebugInfo/ARM/ropi.ll:21
+!1 = distinct !DIGlobalVariable(name: "global", scope: !2, file: !5, line: 1, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 3948962b454022c2c8de6f67942a9cbd1f0351a0)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "/tmp/ropi.c", directory: "/tmp")
----------------
amilendra wrote:
> amilendra wrote:
> > I think these version specific content are not needed, and could break after the next version is branched?
> Ah wait. These are fixed information so no specific to the commit or environment. 
Yes.   This is just information about the version of the build compiler used to generate the initial .ll file and is fixed.


================
Comment at: llvm/test/DebugInfo/ARM/ropi.ll:22
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 15.0.0 (https://github.com/llvm/llvm-project.git 3948962b454022c2c8de6f67942a9cbd1f0351a0)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, globals: !4, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "/tmp/ropi.c", directory: "/tmp")
+!4 = !{!0}
----------------
amilendra wrote:
> Environment specific content; could potentially fail on Windows?
> 
This file information is just used to generate the compilation information in the generatede debugging information.
As we are not going to be actually using the debug information to actually debug on the host platform this should not be an issue.


================
Comment at: llvm/test/DebugInfo/ARM/ropi.ll:34
+!14 = !{i32 8, !"sign-return-address-all", i32 0}
+!15 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
+!16 = !{i32 7, !"frame-pointer", i32 2}
----------------
amilendra wrote:
> Do we need to check for these?
> I think this test case can be considerably reduced to use only the bare minimum needed?
I'll remove some of the entries that are not required which should make the ropi.ll test file more similar to the existing rwpi.ll test file.
The additional entries are because I used a newer version of the compiler to generate the initial ropi.ll file.


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

https://reviews.llvm.org/D126361