Skip to content

Commit 815055b

Browse files
authored
[flag] Update the lifetime markers flag to correctly handle multiple instances (#5283)
The current behavior of the lifetime markers flags is to allow the `-disable-lifetime-markers` flag to override the enable flag no matter where it occurs. This means that if you write dxc -disable-lifetime-markers ... -enable-lifetime-markers the lifetime markers will be disabled. This works differently from most clang flags which allow the last version of the enable/disable flag to win. This PR modifies these flags so that when there are multiple occurences of enable/disable the last one will win. It is not clear if the original behavior was intentional or just a mis-understanding of the flag API. There were no tests enforcing the original behavior.
1 parent 7c77439 commit 815055b

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

lib/DxcSupport/HLSLOptions.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,9 +779,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
779779
opts.ResMayAlias = Args.hasFlag(OPT_res_may_alias_, OPT_INVALID, opts.ResMayAlias);
780780
opts.ForceZeroStoreLifetimes = Args.hasFlag(OPT_force_zero_store_lifetimes, OPT_INVALID, false);
781781
// Lifetime markers on by default in 6.6 unless disabled explicitly
782-
opts.EnableLifetimeMarkers = Args.hasFlag(OPT_enable_lifetime_markers, OPT_INVALID,
783-
DXIL::CompareVersions(Major, Minor, 6, 6) >= 0) &&
784-
!Args.hasFlag(OPT_disable_lifetime_markers, OPT_INVALID, false);
782+
opts.EnableLifetimeMarkers = Args.hasFlag(OPT_enable_lifetime_markers, OPT_disable_lifetime_markers,
783+
DXIL::CompareVersions(Major, Minor, 6, 6) >= 0);
785784
opts.ForceDisableLocTracking =
786785
Args.hasFlag(OPT_fdisable_loc_tracking, OPT_INVALID, false);
787786
opts.NewInlining =
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// This test checks that the lifetime markers flag works correctly when
2+
// multiple instances are given on the command line.
3+
//
4+
// The shader here is not important except that it triggers the insertion
5+
// of lifetime markers when they are enabled.
6+
7+
// Default value is enabled for 6.6.
8+
// RUN: %dxc /Tvs_6_6 %s | FileCheck %s -check-prefix=ENABLE
9+
10+
// Explicitly enabling the flag should enable lifetime markers.
11+
// RUN: %dxc /Tvs_6_6 %s -enable-lifetime-markers | FileCheck %s -check-prefix=ENABLE
12+
13+
// Explicitly disabling the flag should disable lifetime markers.
14+
// RUN: %dxc /Tvs_6_6 %s -disable-lifetime-markers | FileCheck %s -check-prefix=DISABLE
15+
16+
// When both enable and disable flags are given the last occurence should win.
17+
// RUN: %dxc /Tvs_6_6 %s -enable-lifetime-markers -disable-lifetime-markers | FileCheck %s -check-prefix=DISABLE
18+
// RUN: %dxc /Tvs_6_6 %s -disable-lifetime-markers -enable-lifetime-markers | FileCheck %s -check-prefix=ENABLE
19+
20+
// ENABLE: define void @main()
21+
// ENABLE: call void @llvm.lifetime.start
22+
23+
// DISABLE: define void @main()
24+
// DISABLE-NOT: call void @llvm.lifetime.start
25+
26+
void foo() {};
27+
28+
[RootSignature("DescriptorTable(UAV(u0, numDescriptors=10))")]
29+
float main(int N : A, float X : X) : Z {
30+
float Arr[64];
31+
32+
foo();
33+
Arr[N] = 0;
34+
foo();
35+
36+
return Arr[X];
37+
}

0 commit comments

Comments
 (0)