Skip to content

Commit 437ae1e

Browse files
authored
Re-enable a swath of DXIL tests for *nix builds (microsoft#3888)
These were disabled at a point that the DXC CI didn't reject builds for failing on Linux, now that they do, we can enable these tests sure in the knowledge that any changes to them that break on Linux will be detected and fixed quickly Fixed *nix error code return. The previous caused an infinite loop of retries. This definition more accurately identifies the error that has occurred as not benefiting from repeated attempts Skip tests and filechecker elements that entirely depend on reflection or linker interfaces that aren't implemented on *nix yet. Correct capitalization in a few places to satisfy a case-sensitive filesystem Detect directories from test include handler. *nix virtual filesystem will try to load the file using the include handler when performing stat() on a directory. The test include handler is pretty basic and would provide the next file, leaving it without a file to offer when the real include came along. By detecting a name that looks like a dir and returning failure, it correctly indicates that the file isn't found by the handler and saves the virtual file for when its needed. Don't mark non-debug builds as such. The DEBUG define determines which tests get run. Some of which depend on consistencies that can only be relied upon in debug builds. So DEBUG is disabled for all non-debug builds. Correct free of new() memory. Incidental regex allocations were changed for HLSL to use the global new() operator, but were still freed using the standard free(), but replacing the default free() with regex_free() allows the default new() operator to be paired with the default delete() operator Correct misnamed test flag. The *nix flag was mistakenly called InputFile instead of InputPath. By renaming it, the ManualFile test can target a specific HLSL file. Fix misused ArrayRef in legacy GV replacement. The replacement of the GV with the legacy form used in libraries collected the arguments into an arrayref without an array to ref, which caused garbage in release builds. Also moved to using end vars for loops where the elements can get deleted during the loop. Fix *nix conversion to utf8. When an unknown codepage is found, it tries to convert to utf16, but that isn't' actually helpful for *nix for a shader codepage derived from the BOM, which is correctly identified as utf32
1 parent 2bd9843 commit 437ae1e

File tree

13 files changed

+104
-80
lines changed

13 files changed

+104
-80
lines changed

cmake/modules/HandleLLVMOptions.cmake

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ int main() { return (float)x; }"
5656
endif()
5757

5858
if( LLVM_ENABLE_ASSERTIONS )
59-
# MSVC doesn't like _DEBUG on release builds. See PR 4379.
60-
if( NOT MSVC )
61-
add_definitions( -D_DEBUG )
62-
endif()
6359
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDBG") # HLSL Change
6460
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -UNDEBUG") # HLSL Change
6561
if (0) # HLSL Change Starts

include/dxc/Support/WinAdapter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@
118118
#define ERROR_OUT_OF_STRUCTURES ENOMEM
119119
#define ERROR_NOT_CAPABLE EPERM
120120
#define ERROR_NOT_FOUND ENOTSUP
121-
#define ERROR_UNHANDLED_EXCEPTION EINTR
121+
#define ERROR_UNHANDLED_EXCEPTION EBADF
122122

123123
// Used by HRESULT <--> WIN32 error code conversion
124124
#define SEVERITY_ERROR 1

include/dxc/Test/WEXAdapter.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@
6161

6262
#define VERIFY_IS_GREATER_THAN_OR_EQUAL(greater, less) EXPECT_GE(greater, less)
6363

64+
#define VERIFY_IS_GREATER_THAN_2(greater, less) EXPECT_GT(greater, less)
65+
#define VERIFY_IS_GREATER_THAN_3(greater, less, msg) EXPECT_GT(greater, less) << msg
66+
#define VERIFY_IS_GREATER_THAN(...) MACRO_N(VERIFY_IS_GREATER_THAN_, __VA_ARGS__)
67+
68+
#define VERIFY_IS_LESS_THAN_2(greater, less) EXPECT_LT(greater, less)
69+
#define VERIFY_IS_LESS_THAN_3(greater, less, msg) EXPECT_LT(greater, less) << msg
70+
#define VERIFY_IS_LESS_THAN(...) MACRO_N(VERIFY_IS_LESS_THAN_, __VA_ARGS__)
71+
6472
#define VERIFY_WIN32_BOOL_SUCCEEDED_1(expr) EXPECT_TRUE(expr)
6573
#define VERIFY_WIN32_BOOL_SUCCEEDED_2(expr, msg) EXPECT_TRUE(expr) << msg
6674
#define VERIFY_WIN32_BOOL_SUCCEEDED(...) MACRO_N(VERIFY_WIN32_BOOL_SUCCEEDED_, __VA_ARGS__)

lib/DxcSupport/FileIOHelper.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,9 @@ static unsigned CharSizeFromCodePage(UINT32 codePage) {
217217
// We do not handle translation from these code page values.
218218
static bool IsUnsupportedUtfCodePage(UINT32 codePage) {
219219
switch (codePage) {
220+
#ifdef _WIN32
220221
case CP_UTF32LE:
222+
#endif
221223
case CP_UTF32BE:
222224
case CP_UTF16BE:
223225
return true;
@@ -473,7 +475,11 @@ static HRESULT CodePageBufferToUtf8(UINT32 codePage, LPCVOID bufferPointer,
473475
CDxcMallocHeapPtr<WCHAR> utf16NewCopy(pMalloc);
474476
UINT32 utf16CharCount = 0;
475477
const WCHAR *utf16Chars = nullptr;
478+
#if _WIN32
476479
if (codePage == CP_UTF16) {
480+
#else
481+
if (codePage == CP_UTF16 || codePage == CP_UTF32LE) {
482+
#endif
477483
if (!IsSizeWcharAligned(bufferSize))
478484
throw hlsl::Exception(DXC_E_STRING_ENCODING_FAILED,
479485
"Error in encoding argument specified");

lib/HLSL/DxilCondenseResources.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1821,18 +1821,17 @@ bool UpdateStructTypeForLegacyLayout(DxilResourceBase &Res,
18211821
Function *NF = hlslOP->GetOpFunc(hlsl::OP::OpCode::CreateHandleForLib, UpdatedST);
18221822

18231823
// Replace old GV.
1824-
for (auto UserIt = Symbol->user_begin(); UserIt != Symbol->user_end();) {
1824+
for (auto UserIt = Symbol->user_begin(), userEnd = Symbol->user_end(); UserIt != userEnd;) {
18251825
Value *User = *(UserIt++);
18261826

18271827
if (LoadInst *ldInst = dyn_cast<LoadInst>(User)) {
18281828
if (!ldInst->user_empty()) {
18291829
IRBuilder<> Builder = IRBuilder<>(ldInst);
18301830
LoadInst *newLoad = Builder.CreateLoad(NewGV);
1831-
ArrayRef<Value *> args = {hlslOP->GetI32Const((unsigned)hlsl::OP::OpCode::CreateHandleForLib), newLoad};
1831+
Value *args[] = {hlslOP->GetI32Const((unsigned)hlsl::OP::OpCode::CreateHandleForLib), newLoad};
18321832

1833-
for (auto user = ldInst->user_begin(); user != ldInst->user_end();) {
1833+
for (auto user = ldInst->user_begin(), E = ldInst->user_end(); user != E;) {
18341834
CallInst *CI = cast<CallInst>(*(user++));
1835-
18361835
CallInst *newCI = CallInst::Create(NF, args, "", CI);
18371836
CI->replaceAllUsesWith(newCI);
18381837
CI->eraseFromParent();
@@ -1945,7 +1944,7 @@ void ReplaceResourceUserWithHandle(
19451944
DxilResource &res,
19461945
LoadInst *load, Value *handle)
19471946
{
1948-
for (auto resUser = load->user_begin(); resUser != load->user_end();) {
1947+
for (auto resUser = load->user_begin(), E = load->user_end(); resUser != E;) {
19491948
Value *V = *(resUser++);
19501949
CallInst *CI = dyn_cast<CallInst>(V);
19511950
DxilInst_CreateHandleForLib createHandle(CI);

lib/Support/regfree.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ llvm_regfree(llvm_regex_t *preg)
6161
g->magic = 0; /* mark it invalid */
6262

6363
if (g->strip != NULL)
64-
free((char *)g->strip);
64+
regex_free((char *)g->strip);
6565
if (g->sets != NULL)
66-
free((char *)g->sets);
66+
regex_free((char *)g->sets);
6767
if (g->setbits != NULL)
68-
free((char *)g->setbits);
68+
regex_free((char *)g->setbits);
6969
if (g->must != NULL)
70-
free(g->must);
71-
free((char *)g);
70+
regex_free(g->must);
71+
regex_free((char *)g);
7272
}

tools/clang/unittests/HLSL/CMakeLists.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,12 @@ set(HLSL_IGNORE_SOURCES
5959
PixTest.cpp
6060
RewriterTest.cpp
6161
ShaderOpTest.cpp
62-
DxilContainerTest.cpp
63-
ValidationTest.cpp
64-
CompilerTest.cpp
6562
)
6663

6764
add_clang_unittest(clang-hlsl-tests
6865
AllocatorTest.cpp
66+
CompilerTest.cpp
67+
DxilContainerTest.cpp
6968
DxilModuleTest.cpp
7069
DxilResourceTests.cpp
7170
DXIsenseTest.cpp
@@ -77,6 +76,7 @@ add_clang_unittest(clang-hlsl-tests
7776
OptionsTest.cpp
7877
SystemValueTest.cpp
7978
TestMain.cpp
79+
ValidationTest.cpp
8080
VerifierTest.cpp
8181
)
8282

tools/clang/unittests/HLSL/CompilerTest.cpp

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,19 @@
2525
#include "dxc/DxilContainer/DxilContainer.h"
2626
#include "dxc/Support/WinIncludes.h"
2727
#include "dxc/dxcapi.h"
28-
#include "dxc/dxcpix.h"
2928
#ifdef _WIN32
29+
#include "dxc/dxcpix.h"
3030
#include <atlfile.h>
3131
#include <d3dcompiler.h>
3232
#include "dia2.h"
33-
#endif
33+
#else // _WIN32
34+
#ifndef __ANDROID__
35+
#include <execinfo.h>
36+
#define CaptureStackBackTrace(FramesToSkip, FramesToCapture, BackTrace, \
37+
BackTraceHash) \
38+
backtrace(BackTrace, FramesToCapture)
39+
#endif // __ANDROID__
40+
#endif // _WIN32
3441

3542
#include "dxc/Test/HLSLTestData.h"
3643
#include "dxc/Test/HlslTestUtils.h"
@@ -53,6 +60,22 @@
5360
using namespace std;
5461
using namespace hlsl_test;
5562

63+
64+
// Examines a virtual filename to determine if it looks like a dir
65+
// Based on whether the final path segment contains a dot
66+
// Not ironclad, but good enough for testing
67+
static bool LooksLikeDir(std::wstring fileName) {
68+
for(int i = fileName.size() - 1; i > -1; i--) {
69+
wchar_t ch = fileName[i];
70+
if (ch == L'\\' || ch == L'/')
71+
return true;
72+
if (ch == L'.')
73+
return false;
74+
}
75+
// No divider, no dot, assume file
76+
return false;
77+
}
78+
5679
class TestIncludeHandler : public IDxcIncludeHandler {
5780
DXC_MICROCOM_REF_FIELD(m_dwRef)
5881
public:
@@ -94,7 +117,15 @@ class TestIncludeHandler : public IDxcIncludeHandler {
94117
_In_ LPCWSTR pFilename, // Filename as written in #include statement
95118
_COM_Outptr_ IDxcBlob **ppIncludeSource // Resultant source object for included file
96119
) override {
97-
CallInfos.push_back(LoadSourceCallInfo(pFilename));
120+
121+
LoadSourceCallInfo callInfo = LoadSourceCallInfo(pFilename);
122+
123+
// *nix will call stat() on dirs, which attempts to find it here
124+
// This loader isn't meant for dirs, so return failure
125+
if (LooksLikeDir(callInfo.Filename))
126+
return m_defaultErrorCode;
127+
128+
CallInfos.push_back(callInfo);
98129

99130
*ppIncludeSource = nullptr;
100131
if (callIndex >= CallResults.size()) {
@@ -2835,8 +2866,10 @@ struct InstrumentedHeapMalloc : public IMalloc {
28352866
// breakpoint for i failure on NN alloc - m_FailAlloc == 1+VAL && m_AllocCount == NN
28362867
// breakpoint for happy path for NN alloc - m_AllocCount == NN
28372868
P->AllocAtCount = m_AllocCount;
2869+
#ifndef __ANDROID__
28382870
if (CaptureStacks)
28392871
P->AllocFrameCount = CaptureStackBackTrace(1, StackFrameCount, P->AllocFrames, nullptr);
2872+
#endif // __ANDROID__
28402873
P->Size = cb;
28412874
P->Self = P;
28422875
return P + 1;
@@ -2862,9 +2895,11 @@ struct InstrumentedHeapMalloc : public IMalloc {
28622895
m_Size -= P->Size;
28632896
P->Entry.Flink->Blink = P->Entry.Blink;
28642897
P->Entry.Blink->Flink = P->Entry.Flink;
2898+
#ifndef __ANDROID__
28652899
if (CaptureStacks)
28662900
P->FreeFrameCount =
28672901
CaptureStackBackTrace(1, StackFrameCount, P->FreeFrames, nullptr);
2902+
#endif // __ANDROID__
28682903
}
28692904

28702905
virtual SIZE_T STDMETHODCALLTYPE GetSize(
@@ -2896,7 +2931,12 @@ struct InstrumentedHeapMalloc : public IMalloc {
28962931

28972932
#if _ITERATOR_DEBUG_LEVEL==0
28982933
// CompileWhenNoMemThenOOM can properly detect leaks only when debug iterators are disabled
2934+
#ifdef _WIN32
28992935
TEST_F(CompilerTest, CompileWhenNoMemThenOOM) {
2936+
#else
2937+
// Disabled it is ignored above
2938+
TEST_F(CompilerTest, DISABLED_CompileWhenNoMemThenOOM) {
2939+
#endif
29002940
WEX::TestExecution::SetVerifyOutput verifySettings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);
29012941

29022942
CComPtr<IDxcBlobEncoding> pSource;
@@ -3391,6 +3431,7 @@ TEST_F(CompilerTest, CodeGenVectorAtan2) {
33913431
CodeGenTestCheck(L"atan2_vector_argument.hlsl");
33923432
}
33933433

3434+
#ifdef _WIN32 // Reflection unsupported
33943435
TEST_F(CompilerTest, LibGVStore) {
33953436
CComPtr<IDxcCompiler> pCompiler;
33963437
CComPtr<IDxcOperationResult> pResult;
@@ -3466,6 +3507,7 @@ TEST_F(CompilerTest, LibGVStore) {
34663507
std::wstring Text = BlobToUtf16(pTextBlob);
34673508
VERIFY_ARE_NOT_EQUAL(std::wstring::npos, Text.find(L"store"));
34683509
}
3510+
#endif // WIN32 - Reflection unsupported
34693511

34703512
TEST_F(CompilerTest, PreprocessWhenValidThenOK) {
34713513
CComPtr<IDxcCompiler> pCompiler;
@@ -3739,13 +3781,15 @@ TEST_F(CompilerTest, DISABLED_ManualFileCheckTest) {
37393781
}
37403782

37413783

3784+
#ifdef _WIN32 // Reflection unsupported
37423785
TEST_F(CompilerTest, CodeGenHashStability) {
37433786
CodeGenTestCheckBatchHash(L"");
37443787
}
37453788

37463789
TEST_F(CompilerTest, BatchD3DReflect) {
37473790
CodeGenTestCheckBatchDir(L"d3dreflect");
37483791
}
3792+
#endif // WIN32 - Reflection unsupported
37493793

37503794
TEST_F(CompilerTest, BatchDxil) {
37513795
CodeGenTestCheckBatchDir(L"dxil");
@@ -3772,7 +3816,7 @@ TEST_F(CompilerTest, BatchValidation) {
37723816
}
37733817

37743818
TEST_F(CompilerTest, BatchPIX) {
3775-
CodeGenTestCheckBatchDir(L"PIX");
3819+
CodeGenTestCheckBatchDir(L"pix");
37763820
}
37773821

37783822
TEST_F(CompilerTest, BatchSamples) {

tools/clang/unittests/HLSL/DxilContainerTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ class DxilContainerTest : public ::testing::Test {
122122
UINT32 codePage, _Outptr_ IDxcBlobEncoding **ppBlob) {
123123
CComPtr<IDxcLibrary> library;
124124
IFT(m_dllSupport.CreateInstance(CLSID_DxcLibrary, &library));
125-
IFT(library->CreateBlobWithEncodingFromPinned((LPBYTE)data, size, codePage,
125+
IFT(library->CreateBlobWithEncodingFromPinned(data, size, codePage,
126126
ppBlob));
127127
}
128128

@@ -351,9 +351,7 @@ class DxilContainerTest : public ::testing::Test {
351351
VERIFY_ARE_EQUAL(testZ, baseZ);
352352
}
353353
}
354-
#endif // _WIN32 - Reflection unsupported
355354

356-
#ifdef _WIN32 // - Reflection unsupported
357355
HRESULT CompileFromFile(LPCWSTR path, bool useDXBC,
358356
UINT fxcFlags, IDxcBlob **ppBlob) {
359357
std::vector<FileRunCommandPart> parts;
@@ -793,6 +791,7 @@ TEST_F(DxilContainerTest, CompileWhenSigSquareThenIncludeSplit) {
793791
#endif
794792
}
795793

794+
#ifdef _WIN32 // - Reflection unsupported
796795
TEST_F(DxilContainerTest, CompileAS_CheckPSV0) {
797796
if (m_ver.SkipDxilVersion(1, 5)) return;
798797
const char asSource[] =
@@ -1553,6 +1552,7 @@ TEST_F(DxilContainerTest, DxcUtils_CreateReflection) {
15531552
}
15541553
}
15551554
}
1555+
#endif // _WIN32 - Reflection unsupported
15561556

15571557
TEST_F(DxilContainerTest, CompileWhenOKThenIncludesFeatureInfo) {
15581558
CComPtr<IDxcCompiler> pCompiler;

tools/clang/unittests/HLSL/HLSLTestOptions.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ namespace testOptions {
3737
ARGOP(ExperimentalShaders)\
3838
ARGOP(DebugLayer)\
3939
ARGOP(SuitePath)\
40-
ARGOP(InputFile)
40+
ARGOP(InputPath)
4141

4242
ARG_LIST(ARG_DECLARE)
4343

tools/clang/unittests/HLSL/ValidationTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ class ValidationTest : public ::testing::Test {
349349

350350
CA2W shWide(pShaderModel, CP_UTF8);
351351

352-
wchar_t *pEntryName = L"main";
352+
const wchar_t *pEntryName = L"main";
353353

354354
llvm::StringRef stage;
355355
unsigned RequiredDxilMajor = 1, RequiredDxilMinor = 0;

tools/clang/unittests/HLSLTestLib/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ add_clang_library(HLSLTestLib
1212
else(WIN32)
1313
set(HLSL_IGNORE_SOURCES
1414
D3DReflectionDumper.cpp
15-
FileCheckerTest.cpp
1615
)
1716
add_clang_library(HLSLTestLib
1817
DxcTestUtils.cpp
18+
FileCheckerTest.cpp
1919
FileCheckForTest.cpp
2020
)
2121
include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)

0 commit comments

Comments
 (0)