This is an archive of the discontinued LLVM Phabricator instance.

DebugInfo: Move missing-abstract-variable.ll test from Generic to X86
ClosedPublic

Authored by tstellar on Sep 14 2021, 8:26 PM.

Download Raw Diff

Details

Reviewers

jmorse
echristo
mehdi_amini
kwk
dblaikie
probinson

Commits

rG247dedea4e03: DebugInfo: Move missing-abstract-variable.ll test from Generic to X86

Summary

This test is XFAIL'd on most arches, and seems too fragile to be run on non-X86.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed

	Time	Test
	60,050 ms	x64 debian > MLIR.Examples/standalone::test.toy
	60,040 ms	x64 debian > libFuzzer.libFuzzer::value-profile-load.test

Event Timeline

tstellar created this revision.Sep 14 2021, 8:26 PM

Herald added a subscriber: shchenz. · View Herald TranscriptSep 14 2021, 8:26 PM

tstellar requested review of this revision.Sep 14 2021, 8:26 PM

Herald added a project: Restricted Project. · View Herald TranscriptSep 14 2021, 8:26 PM

Harbormaster completed remote builds in B123955: Diff 372625.Sep 14 2021, 8:26 PM

when llvm is configured with LLVM_DEFAULT_TARGET_TRIPLE set to a non-empty value,

Do you mean an empty value? I believe this is set to the host by default.

In D109806#3001029, @mehdi_amini wrote:

when llvm is configured with LLVM_DEFAULT_TARGET_TRIPLE set to a non-empty value,

Do you mean an empty value? I believe this is set to the host by default.

Yes, you are correct, there is a default value form this variable which comes from config.guess. So I guess this means this test is sensitive to what the target triple is, because it's failing for me when I set the default triple to ppc64le-redhat-linux-gnu, but not when it uses the default triple of powerpc64le-unknown-linux-gnu.

tstellar edited the summary of this revision. (Show Details)Sep 14 2021, 9:06 PM

Herald added a subscriber: steven.zhang. · View Herald TranscriptSep 14 2021, 9:06 PM

LGTM

This revision is now accepted and ready to land.Sep 23 2021, 7:49 AM

Also, the test passes for me when LLVM is configured to use the default triple of powerpc64le-unknown-linux-gnu, but fails when I configure the default triple to be ppc64le-redhat-linux-gnu.

Hmmm. Possibly the longer term fix is for this test to
a) Not be in the 'Generic' directory, or
b) Become a MIR test instead of exercising instruction selection as part of the test too.

(Adding an XFail in the meantime seems fine to me, especially as powerpc64 is already known to have this issue).

This needs rebasing...

This revision now requires changes to proceed.Nov 9 2021, 6:03 AM

Move test to X86 directory.

Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2022, 3:50 PM

Herald added subscribers: • pcwang-thead, s.egerton, simoncook, fedor.sergeev. · View Herald Transcript

tstellar retitled this revision from XFAIL missing-abstract-variable.ll test on ppc64le to DebugInfo: Move missing-abstract-variable.ll test from Generic to X86.Oct 31 2022, 3:51 PM

tstellar edited the summary of this revision. (Show Details)

tstellar added a reviewer: dblaikie.

Herald added a subscriber: pengfei. · View Herald TranscriptOct 31 2022, 3:51 PM

In D109806#3018155, @jmorse wrote:

Also, the test passes for me when LLVM is configured to use the default triple of powerpc64le-unknown-linux-gnu, but fails when I configure the default triple to be ppc64le-redhat-linux-gnu.

Hmmm. Possibly the longer term fix is for this test to
a) Not be in the 'Generic' directory, or
b) Become a MIR test instead of exercising instruction selection as part of the test too.

(Adding an XFail in the meantime seems fine to me, especially as powerpc64 is already known to have this issue).

In the mean time, the test has been XFAIL'd on many more arches, so I just moved it to the X86 directory.

Harbormaster completed remote builds in B195380: Diff 472166.Oct 31 2022, 5:50 PM

Seems like a reasonable resolution. Better than continually extending the XFAIL list.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 3 2022, 2:02 PM

This revision was landed with ongoing or failed builds.

Closed by commit rG247dedea4e03: DebugInfo: Move missing-abstract-variable.ll test from Generic to X86 (authored by tstellar). · Explain Why

This revision was automatically updated to reflect the committed changes.

tstellar added a commit: rG247dedea4e03: DebugInfo: Move missing-abstract-variable.ll test from Generic to X86.

Revision Contents

Path

Size

llvm/

test/

DebugInfo/

Generic/

missing-abstract-variable.ll

	X86/
	Generic/

missing-abstract-variable.ll

8 lines

Diff 472166

llvm/test/DebugInfo/Generic/missing-abstract-variable.ll

This file was moved to llvm/test/DebugInfo/X86/missing-abstract-variable.ll.

llvm/test/DebugInfo/X86/missing-abstract-variable.ll

This file was moved from llvm/test/DebugInfo/Generic/missing-abstract-variable.ll.

	; RUN: %llc_dwarf -O0 -filetype=obj < %s \| llvm-dwarfdump -debug-info - \| FileCheck %s			; RUN: %llc_dwarf -mtriple x86_64-gnu-linux -O0 -filetype=obj < %s \| llvm-dwarfdump -debug-info - \| FileCheck %s

	; The formal parameter 'b' for Function 'x' when inlined within 'a' is lost on
	; powerpc64 (and on x86_64 at at least -O2). Presumably this is a SelectionDAG
	; issue.
	; FIXME: arm64 is an alias for aarch64 on macs, apparently?
	; XFAIL: powerpc64, aarch64, arm64, hexagon, riscv, sparc, loongarch

	; Build from the following source with clang -O2.			; Build from the following source with clang -O2.

	; The important details are that 'x's abstract definition is first built during			; The important details are that 'x's abstract definition is first built during
	; the definition of 'b', where the parameter to 'x' is constant and so 'x's 's'			; the definition of 'b', where the parameter to 'x' is constant and so 'x's 's'
	; variable is optimized away. No abstract definition DIE for 's' is constructed.			; variable is optimized away. No abstract definition DIE for 's' is constructed.
	; Then, during 'a' emission, the abstract DbgVariable for 's' is created, but			; Then, during 'a' emission, the abstract DbgVariable for 's' is created, but
	; the abstract DIE isn't (since the abstract definition for 'b' is already			; the abstract DIE isn't (since the abstract definition for 'b' is already
	▲ Show 20 Lines • Show All 166 Lines • Show Last 20 Lines