Page MenuHomePhabricator

[IRSim] Removing length check so first instruction in module is included
ClosedPublic

Authored by AndrewLitteken on Mar 1 2022, 4:58 PM.

Details

Summary

Discussed: https://github.com/llvm/llvm-project/issues/54124

If an instruction is first legal instruction in the module, and is the only legal instruction in its basic block, it will be ignored by the outliner due to a length check inherited from the older version of the outliner that was restricted to outlining within a single basic block. This removes that check, and updates any tests that broke because of it.

Diff Detail

Event Timeline

AndrewLitteken created this revision.Mar 1 2022, 4:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 4:58 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
AndrewLitteken requested review of this revision.Mar 1 2022, 4:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 4:58 PM

What is the code size impact on the test suite?

I measured myself and found that this actually exposes *another* bug.

This IR:

https://godbolt.org/z/Wxq5aEsGT

Compiled with this patch using

~/llvm-project/build/bin/opt -iroutliner /tmp/testcase.ll -o -

Gives us

!dbg attachment points at wrong subprogram for function
!12 = distinct !DISubprogram(name: "outlined_ir_func_0", linkageName: "outlined_ir_func_0", scope: !2, file: !2, type: !3, flags: DIFlagArtificial, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !5, retainedNodes: !4)
i1 ()* @outlined_ir_func_0
  br i1 undef, label %bb5, label %bb13.exitStub, !llvm.loop !13
!14 = !DILocation(line: 1655, column: 5, scope: !15)
!15 = distinct !DISubprogram(name: "snork.outlined", linkageName: "snork.outlined", scope: null, file: !2, type: !3, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !5, retainedNodes: !4)
!15 = distinct !DISubprogram(name: "snork.outlined", linkageName: "snork.outlined", scope: null, file: !2, type: !3, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !5, retainedNodes: !4)
LLVM ERROR: Broken module found, compilation aborted!

I know this is a real bug because I saw it in a much more complex setting than the test suite. :) This change (thankfully?) exposes it. The bug ought to be independent of this patch.

Oh, well I'll add it to the list. It looks like it's due to loop metadata not being stripped, only debug info location info gets stripped at the moment.

paquette accepted this revision.Mar 2 2022, 1:37 PM

I think we can go ahead and land this.

Can you add a new testcase which tests something along the lines of

https://godbolt.org/z/6Pav4hnfh

?

This revision is now accepted and ready to land.Mar 2 2022, 1:37 PM

Oh, good news, for the other issue, I managed to repro without the patch:

https://github.com/llvm/llvm-project/issues/54155

paquette accepted this revision.Mar 3 2022, 9:15 AM

LGTM

This revision was landed with ongoing or failed builds.Mar 13 2022, 9:19 PM
This revision was automatically updated to reflect the committed changes.