Supplement to fix failures for imports with LLVM-17 #1415
Conversation
LLVM-17 expects imports list in retainedNode field of DISubprogram, and flang has completed this step but it's still generating imports list as imports field of DICompilationUnit. Flang is now updated to new IR as expected by LLVM-17.
bryanpkc
requested review from
bryanpkc,
pawosm-arm,
shivaramaarao and
alokkrsharma
|
@Liuyunlong0336 In your commit message you should probably mention that this is an update of the fix initially introduced in 08b46d7, and explain why that commit couldn't catch the case you are fixing. |
|
@alokkrsharma Does this look okay to you? |
| @@ -935,7 +935,6 @@ static const MDTemplate Tmpl_DICompileUnit_ver39[] = { | |||
| { "retainedTypes", NodeField, 0 }, | |||
| { "globals", NodeField, 0 }, | |||
| { "emissionKind", DWEmissionField, 0 }, | |||
There was a problem hiding this comment.
Shouldn't a new array Tmpl_DICompileUnit_ver170 and Tmpl_DICompileUnit_ver39 kept intact for backward compatibility? Although our inner policy is kind of 'no going back', others may not be so happy about it.
There was a problem hiding this comment.
I'm pretty sure that LLVM 17 still accepts imports as an optional field. There should be no need to delete this. It should be feasible for Classic Flang to generate this field as imports: null or imports: !2 where !2 is empty.
|
Upon further review, I don't think this PR is necessary. #1414 had fixed the problem properly. |
LLVM-17 expects imports list in retainedNode field of DISubprogram, and flang has completed this step but it's still generating imports list as imports field of DICompilationUnit.
Flang is now updated to new IR as expected by LLVM-17.