Skip to content

Commit d37451a

Browse files
YuriPlyakhinYuriPlyakhin
committed
addressed feedback
1 parent 25ea27e commit d37451a

File tree

4 files changed

+42
-32
lines changed

4 files changed

+42
-32
lines changed

‎clang/test/Driver/clang-offload-wrapper-exe-preview.cpp‎

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,39 @@
1-
// REQUIRES: system-linux || system-windows
2-
31
// End-to-end clang-offload-wrapper executable test: check that -batch options
42
// works, and that the tool generates data properly accessible at runtime.
53

6-
// --- Prepare test data
7-
// - create the first binary image
4+
// Prepare test data. Create the first binary image.
85
// RUN: echo -e -n 'device binary image1\n' > %t.bin
96
// RUN: echo -e -n '[Category1]\nint_prop1=1|10\n[Category2]\nint_prop2=1|20\n' > %t.props
107
// RUN: echo -e -n 'kernel1\nkernel2\n' > %t.sym
118

12-
// - create the second binary image with byte array property values
9+
// Create the second binary image with byte array property values.
1310
// RUN: echo -e -n 'device binary image2\n' > %t_1.bin
1411
// RUN: echo -e -n '[Category3]\n' > %t_1.props
1512
// RUN: echo -e -n 'kernel1=2|IAAAAAAAAAQA\n' >> %t_1.props
1613
// RUN: echo -e -n 'kernel2=2|oAAAAAAAAAw///3/wB\n' >> %t_1.props
1714

18-
// - create the batch file input for the wrapper
15+
// Create the batch file input for the wrapper.
1916
// RUN: echo '[Code|Properties|Symbols]' > %t.batch
2017
// RUN: echo %t.bin"|"%t.props"|"%t.sym >> %t.batch
2118
// RUN: echo %t_1.bin"|"%t_1.props"|" >> %t.batch
22-
// --- Generate "gold" output
19+
20+
// Generate "gold" output.
2321
// RUN: cat %t.bin %t.props %t.sym > %t.all
24-
// --- Create the wrapper object
25-
// -host omitted - generate object for the host triple:
22+
23+
// Create the wrapper object.
2624
#ifdef __INTEL_PREVIEW_BREAKING_CHANGES
27-
/// TODO: Remove -fpreview-breaking-changes from the command line, when removing the macro
25+
/// TODO: Remove -fpreview-breaking-changes from the command line, when removing the macro.
2826
// RUN: clang-offload-wrapper -kind=sycl -target=TARGET -format=native -batch %t.batch -o %t.wrapped.bc -fpreview-breaking-changes
2927
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
3028
// RUN: llc --filetype=obj %t.wrapped.bc -o %t.wrapped.o
31-
// --- Compile & link the test with the wrapper
29+
30+
// Compile & link the test with the wrapper.
3231
// RUN: %clangxx %t.wrapped.o %s -o %t.batch.exe -v
33-
// --- Run and check ignoring white spaces
32+
33+
// Run and check ignoring white spaces.
3434
// RUN: %t.batch.exe > %t.batch.exe.out
3535
// RUN: diff -b %t.batch.exe.out %t.all
3636

37-
#include <cassert>
3837
#include <cstdint>
3938
#include <cstring>
4039
#include <iostream>
@@ -53,26 +52,26 @@ struct _pi_offload_entry_struct {
5352
typedef _pi_offload_entry_struct *_pi_offload_entry;
5453

5554
struct _pi_device_binary_property_struct {
56-
char *Name; // null-terminated property name
57-
void *ValAddr; // address of property value
58-
uint32_t Type; // pi_property_type
59-
uint64_t ValSize; // size of property value in bytes
55+
char *Name; // Null-terminated property name.
56+
void *ValAddr; // Address of property value.
57+
uint32_t Type; // pi_property_type.
58+
uint64_t ValSize; // Size of property value in bytes.
6059
};
6160

6261
typedef _pi_device_binary_property_struct *pi_device_binary_property;
6362

6463
struct _pi_device_binary_property_set_struct {
65-
char *Name; // the name
66-
pi_device_binary_property PropertiesBegin; // array start
67-
pi_device_binary_property PropertiesEnd; // array end
64+
char *Name; // The name.
65+
pi_device_binary_property PropertiesBegin; // Array start.
66+
pi_device_binary_property PropertiesEnd; // Array end.
6867
};
6968

7069
typedef _pi_device_binary_property_set_struct *pi_device_binary_property_set;
7170

7271
struct pi_device_binary_struct {
7372
uint16_t Version;
74-
uint8_t Kind; // 4 for SYCL
75-
uint8_t Format; // 1 for native
73+
uint8_t Kind; // 4 for SYCL.
74+
uint8_t Format; // 1 for native.
7675
const char *DeviceTargetSpec;
7776
const char *CompileOptions;
7877
const char *LinkOptions;
@@ -96,7 +95,7 @@ typedef pi_device_binaries_struct *pi_device_binaries;
9695

9796
static pi_device_binaries BinDesc = nullptr;
9897

99-
// Wrapper object has code which calls these 2 functions below
98+
// Wrapper object has code which calls these 2 functions below.
10099
extern "C" void __sycl_register_lib(pi_device_binaries desc) {
101100
BinDesc = desc;
102101
}
@@ -149,9 +148,9 @@ static int dumpBinary0() {
149148
ASSERT(Bin->Kind == 4, "Bin->Kind");
150149
ASSERT(Bin->Format == 1, "Bin->Format");
151150

152-
// dump code
151+
// Dump code.
153152
std::cout << getString(Bin->BinaryStart, Bin->BinaryEnd);
154-
// dump properties
153+
// Dump properties.
155154
for (pi_device_binary_property_set PropSet = Bin->PropertySetsBegin; PropSet != Bin->PropertySetsEnd; ++PropSet) {
156155
std::cout << "[" << PropSet->Name << "]"
157156
<< "\n";
@@ -161,7 +160,7 @@ static int dumpBinary0() {
161160
std::cout << Prop->Name << "=" << Prop->Type << "|" << getInt(&Prop->ValSize) << "\n";
162161
}
163162
}
164-
// dump symbols
163+
// Dump symbols.
165164
for (_pi_offload_entry Entry = Bin->EntriesBegin; Entry != Bin->EntriesEnd; ++Entry)
166165
std::cout << Entry->name << "\n";
167166
return 0;
@@ -191,7 +190,7 @@ static int checkBinary1() {
191190
int Cnt = 0;
192191

193192
for (pi_device_binary_property Prop = PropSet->PropertiesBegin; Prop != PropSet->PropertiesEnd; ++Prop, ++Cnt) {
194-
ASSERT(Prop->Type == 2, "Prop->Type"); // must be a byte array
193+
ASSERT(Prop->Type == 2, "Prop->Type"); // Must be a byte array.
195194
char *Ptr = reinterpret_cast<char *>(Prop->ValAddr);
196195
int Cmp = std::memcmp(Prop->ValAddr, GoldArrays[Cnt].Ptr, GoldArrays[Cnt].Size);
197196
ASSERT(Cmp == 0, "byte array property");

‎clang/tools/clang-offload-wrapper/ClangOffloadWrapper.cpp‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,6 @@ static constexpr char COL_CODE[] = "Code";
8282
static constexpr char COL_SYM[] = "Symbols";
8383
static constexpr char COL_PROPS[] = "Properties";
8484
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
85-
/// TODO: here and below in addition to removing code under macro
86-
/// #ifndef __INTEL_PREVIEW_BREAKING_CHANGES
87-
/// also remove conditional code under `!PreviewBreakingChanges` option.
8885
static constexpr char COL_MANIFEST[] = "Manifest";
8986
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
9087

@@ -605,6 +602,7 @@ class BinaryWrapper {
605602

606603
StructType *getSyclDeviceImageTy() {
607604
if (!SyclImageTy) {
605+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
608606
if (!PreviewBreakingChanges) {
609607
SyclImageTy = StructType::create(
610608
{
@@ -625,6 +623,7 @@ class BinaryWrapper {
625623
},
626624
"__tgt_device_image");
627625
} else {
626+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
628627
SyclImageTy = StructType::create(
629628
{
630629
Type::getInt16Ty(C), // Version
@@ -641,7 +640,9 @@ class BinaryWrapper {
641640
getPtrTy() // PropertySetEnd
642641
},
643642
"__tgt_device_image");
643+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
644644
}
645+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
645646
}
646647
return SyclImageTy;
647648
}
@@ -2011,14 +2012,20 @@ int main(int argc, const char **argv) {
20112012
// ID != 0 signal that a new image(s) must be added
20122013
if (ID != 0) {
20132014
// create an image instance using current state
2015+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
20142016
if (!PreviewBreakingChanges && CurInputGroup.size() > 2) {
20152017
reportError(
20162018
createStringError(errc::invalid_argument,
20172019
"too many inputs for a single binary image, "
20182020
"<binary file> <manifest file>{opt}expected"));
20192021
return 1;
20202022
}
2021-
if (PreviewBreakingChanges && CurInputGroup.size() > 1) {
2023+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
2024+
if (
2025+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
2026+
PreviewBreakingChanges &&
2027+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
2028+
CurInputGroup.size() > 1) {
20222029
reportError(createStringError(errc::invalid_argument,
20232030
"too many inputs for a single binary "
20242031
"image, <binary file> expected"));

‎llvm/lib/Frontend/Offloading/SYCLOffloadWrapper.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ struct Wrapper {
179179
/// };
180180
/// \endcode
181181
StructType *getSyclDeviceImageTy() {
182+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
182183
if (!PreviewBreakingChanges) {
183184
return StructType::create(
184185
{
@@ -199,6 +200,7 @@ struct Wrapper {
199200
},
200201
"__sycl.tgt_device_image");
201202
} else {
203+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
202204
return StructType::create(
203205
{
204206
Type::getInt16Ty(C), // Version
@@ -215,7 +217,9 @@ struct Wrapper {
215217
PointerType::getUnqual(C) // PropertySetEnd
216218
},
217219
"__sycl.tgt_device_image");
220+
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
218221
}
222+
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
219223
}
220224

221225
/// Creates a structure for SYCL specific binary descriptor type. Corresponds

‎sycl/unittests/helpers/MockDeviceImage.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ class MockDeviceImage {
337337
SYCL_DEVICE_BINARY_TYPE_SPIRV,
338338
__SYCL_DEVICE_BINARY_TARGET_SPIRV64, "", "",
339339
#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
340-
{},
340+
{}, // Manifest.
341341
#endif // __INTEL_PREVIEW_BREAKING_CHANGES
342342
std::vector<unsigned char>{1, 2, 3, 4, 5},
343343
internal::LifetimeExtender(std::move(OffloadEntries)),

0 commit comments

Comments
 (0)