Skip to content

Commit 3249bf9

Browse files
authored
JIT: Remove dataSection flexible array (#124373)
Storing pointers and `emitLocation` instances in this `BYTE` flexible array is problematic due to alignment. We could use `alignas`, but the flexible array here is a micro optimization so just go with a simpler representation without the footguns.
1 parent bf62494 commit 3249bf9

File tree

6 files changed

+64
-41
lines changed

6 files changed

+64
-41
lines changed

src/coreclr/jit/codegencommon.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6815,7 +6815,7 @@ void CodeGen::genReportAsyncDebugInfo()
68156815
uint32_t diagNativeOffset = 0;
68166816
if (genAsyncResumeInfoTable != nullptr)
68176817
{
6818-
emitLocation& emitLoc = ((emitLocation*)genAsyncResumeInfoTable->dsCont)[i];
6818+
emitLocation& emitLoc = genAsyncResumeInfoTable->Locations()[i];
68196819
if (emitLoc.Valid())
68206820
{
68216821
diagNativeOffset = emitLoc.CodeOffset(GetEmitter());

src/coreclr/jit/codegenlinear.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@ void CodeGen::genRecordAsyncResume(GenTreeVal* asyncResume)
961961
emitter::dataSection* asyncResumeInfo;
962962
genEmitAsyncResumeInfoTable(&asyncResumeInfo);
963963

964-
((emitLocation*)asyncResumeInfo->dsCont)[index] = emitLocation(GetEmitter());
964+
asyncResumeInfo->Locations()[index] = emitLocation(GetEmitter());
965965
}
966966

967967
/*

src/coreclr/jit/emit.cpp

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7811,14 +7811,14 @@ UNATIVE_OFFSET emitter::emitDataGenBeg(unsigned size, unsigned alignment, var_ty
78117811

78127812
/* Allocate a data section descriptor and add it to the list */
78137813

7814-
dataSection* secDesc = emitDataSecCur = (dataSection*)emitGetMem(roundUp(sizeof(*secDesc) + size));
7814+
dataSection* secDesc = emitDataSecCur = (dataSection*)emitGetMem(sizeof(*secDesc));
7815+
secDesc->dsType = dataSection::data;
7816+
secDesc->Data() = (BYTE*)emitGetMem(size);
78157817

78167818
secDesc->dsSize = size;
78177819

78187820
secDesc->dsAlignment = alignment;
78197821

7820-
secDesc->dsType = dataSection::data;
7821-
78227822
secDesc->dsDataType = dataType;
78237823

78247824
secDesc->dsNext = nullptr;
@@ -7864,14 +7864,14 @@ UNATIVE_OFFSET emitter::emitBBTableDataGenBeg(unsigned numEntries, bool relative
78647864

78657865
/* Allocate a data section descriptor and add it to the list */
78667866

7867-
secDesc = emitDataSecCur = (dataSection*)emitGetMem(roundUp(sizeof(*secDesc) + numEntries * sizeof(BasicBlock*)));
7867+
secDesc = emitDataSecCur = (dataSection*)emitGetMem(sizeof(*secDesc));
7868+
secDesc->dsType = relativeAddr ? dataSection::blockRelative32 : dataSection::blockAbsoluteAddr;
7869+
secDesc->Blocks() = (BasicBlock**)emitGetMem(numEntries * sizeof(BasicBlock*));
78687870

78697871
secDesc->dsSize = emittedSize;
78707872

78717873
secDesc->dsAlignment = elemSize;
78727874

7873-
secDesc->dsType = relativeAddr ? dataSection::blockRelative32 : dataSection::blockAbsoluteAddr;
7874-
78757875
secDesc->dsDataType = TYP_UNKNOWN;
78767876

78777877
secDesc->dsNext = nullptr;
@@ -7905,14 +7905,15 @@ void emitter::emitAsyncResumeTable(unsigned numEntries, UNATIVE_OFFSET* dataSecO
79057905
unsigned emittedSize = sizeof(CORINFO_AsyncResumeInfo) * numEntries;
79067906
emitConsDsc.dsdOffs += emittedSize;
79077907

7908-
dataSection* secDesc = (dataSection*)emitGetMem(roundUp(sizeof(dataSection) + numEntries * sizeof(emitLocation)));
7908+
dataSection* secDesc = (dataSection*)emitGetMem(sizeof(dataSection));
7909+
secDesc->dsType = dataSection::asyncResumeInfo;
7910+
secDesc->Locations() = (emitLocation*)emitGetMem(numEntries * sizeof(emitLocation));
79097911

79107912
for (unsigned i = 0; i < numEntries; i++)
7911-
new (secDesc->dsCont + i * sizeof(emitLocation), jitstd::placement_t()) emitLocation();
7913+
new (&secDesc->Locations()[i], jitstd::placement_t()) emitLocation();
79127914

79137915
secDesc->dsSize = emittedSize;
79147916
secDesc->dsAlignment = TARGET_POINTER_SIZE;
7915-
secDesc->dsType = dataSection::asyncResumeInfo;
79167917
secDesc->dsDataType = TYP_UNKNOWN;
79177918
secDesc->dsNext = nullptr;
79187919

@@ -7949,7 +7950,7 @@ void emitter::emitDataGenData(unsigned offs, const void* data, UNATIVE_OFFSET si
79497950

79507951
assert(emitDataSecCur->dsType == dataSection::data);
79517952

7952-
memcpy(emitDataSecCur->dsCont + offs, data, size);
7953+
memcpy(emitDataSecCur->Data() + offs, data, size);
79537954
}
79547955

79557956
/*****************************************************************************
@@ -7967,7 +7968,7 @@ void emitter::emitDataGenData(unsigned index, BasicBlock* label)
79677968

79687969
assert(emitDataSecCur->dsSize >= emittedElemSize * (index + 1));
79697970

7970-
((BasicBlock**)(emitDataSecCur->dsCont))[index] = label;
7971+
emitDataSecCur->Blocks()[index] = label;
79717972
}
79727973

79737974
/*****************************************************************************
@@ -8011,7 +8012,7 @@ UNATIVE_OFFSET emitter::emitDataGenFind(const void* cnsAddr, unsigned cnsSize, u
80118012
//
80128013
if ((secDesc->dsType == dataSection::data) && (secDesc->dsSize >= cnsSize) && ((curOffs % alignment) == 0))
80138014
{
8014-
if (memcmp(cnsAddr, secDesc->dsCont, cnsSize) == 0)
8015+
if (memcmp(cnsAddr, secDesc->Data(), cnsSize) == 0)
80158016
{
80168017
cnum = curOffs;
80178018

@@ -8393,7 +8394,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
83938394
target_size_t* bDstRW = (target_size_t*)dstRW;
83948395
for (unsigned i = 0; i < numElems; i++)
83958396
{
8396-
BasicBlock* block = ((BasicBlock**)dsc->dsCont)[i];
8397+
BasicBlock* block = dsc->Blocks()[i];
83978398

83988399
// Convert the BasicBlock* value to an IG address
83998400
insGroup* lab = (insGroup*)emitCodeGetCookie(block);
@@ -8424,7 +8425,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
84248425

84258426
for (unsigned i = 0; i < numElems; i++)
84268427
{
8427-
BasicBlock* block = ((BasicBlock**)dsc->dsCont)[i];
8428+
BasicBlock* block = dsc->Blocks()[i];
84288429

84298430
// Convert the BasicBlock* value to an IG address
84308431
insGroup* lab = (insGroup*)emitCodeGetCookie(block);
@@ -8444,7 +8445,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
84448445
CORINFO_AsyncResumeInfo* aDstRW = (CORINFO_AsyncResumeInfo*)dstRW;
84458446
for (size_t i = 0; i < numElems; i++)
84468447
{
8447-
emitLocation* emitLoc = &((emitLocation*)dsc->dsCont)[i];
8448+
emitLocation* emitLoc = &dsc->Locations()[i];
84488449

84498450
// Async call may have been removed very late, after we have introduced suspension/resumption.
84508451
// In those cases just encode null.
@@ -8468,7 +8469,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
84688469
// Simple binary data: copy the bytes to the target
84698470
assert(dsc->dsType == dataSection::data);
84708471

8471-
memcpy(dstRW, dsc->dsCont, dscSize);
8472+
memcpy(dstRW, dsc->Data(), dscSize);
84728473

84738474
#ifdef DEBUG
84748475
if (EMITVERBOSE)
@@ -8477,7 +8478,7 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
84778478

84788479
for (size_t i = 0; i < dscSize; i++)
84798480
{
8480-
printf("%02x ", dsc->dsCont[i]);
8481+
printf("%02x ", dsc->Data()[i]);
84818482
if ((((i + 1) % 16) == 0) && (i + 1 != dscSize))
84828483
{
84838484
printf("\n\t\t\t\t\t");
@@ -8487,10 +8488,10 @@ void emitter::emitOutputDataSec(dataSecDsc* sec, AllocMemChunk* chunks)
84878488
{
84888489
case TYP_FLOAT:
84898490
printf(" ; float %9.6g",
8490-
FloatingPointUtils::convertToDouble(*reinterpret_cast<float*>(&dsc->dsCont)));
8491+
FloatingPointUtils::convertToDouble(*reinterpret_cast<float*>(dsc->Data())));
84918492
break;
84928493
case TYP_DOUBLE:
8493-
printf(" ; double %12.9g", *reinterpret_cast<double*>(&dsc->dsCont));
8494+
printf(" ; double %12.9g", *reinterpret_cast<double*>(dsc->Data()));
84948495
break;
84958496
default:
84968497
break;
@@ -8551,7 +8552,7 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
85518552
printf(labelFormat, "");
85528553
}
85538554

8554-
BasicBlock* block = reinterpret_cast<BasicBlock**>(data->dsCont)[i];
8555+
BasicBlock* block = data->Blocks()[i];
85558556
insGroup* ig = static_cast<insGroup*>(emitCodeGetCookie(block));
85568557

85578558
const char* blockLabel = emitLabelString(ig);
@@ -8617,7 +8618,7 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
86178618
printf(labelFormat, label);
86188619
}
86198620

8620-
emitLocation* emitLoc = &((emitLocation*)data->dsCont)[i];
8621+
emitLocation* emitLoc = &data->Locations()[i];
86218622

86228623
printf("\tdq\t%s\n", resumeStubName);
86238624

@@ -8668,9 +8669,9 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
86688669
{
86698670
printf("\t<Unexpected data size %d (expected >= 4)\n", data->dsSize);
86708671
}
8671-
printf("\tdd\t%08llXh\t", (UINT64) * reinterpret_cast<uint32_t*>(&data->dsCont[i]));
8672+
printf("\tdd\t%08llXh\t", (UINT64) * reinterpret_cast<uint32_t*>(&data->Data()[i]));
86728673
printf("\t; %9.6g",
8673-
FloatingPointUtils::convertToDouble(*reinterpret_cast<float*>(&data->dsCont[i])));
8674+
FloatingPointUtils::convertToDouble(*reinterpret_cast<float*>(&data->Data()[i])));
86748675
i += 4;
86758676
break;
86768677

@@ -8679,21 +8680,21 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
86798680
{
86808681
printf("\t<Unexpected data size %d (expected >= 8)\n", data->dsSize);
86818682
}
8682-
printf("\tdq\t%016llXh", *reinterpret_cast<uint64_t*>(&data->dsCont[i]));
8683-
printf("\t; %12.9g", *reinterpret_cast<double*>(&data->dsCont[i]));
8683+
printf("\tdq\t%016llXh", *reinterpret_cast<uint64_t*>(&data->Data()[i]));
8684+
printf("\t; %12.9g", *reinterpret_cast<double*>(&data->Data()[i]));
86848685
i += 8;
86858686
break;
86868687

86878688
default:
86888689
switch (elemSize)
86898690
{
86908691
case 1:
8691-
printf("\tdb\t%02Xh", *reinterpret_cast<uint8_t*>(&data->dsCont[i]));
8692+
printf("\tdb\t%02Xh", *reinterpret_cast<uint8_t*>(&data->Data()[i]));
86928693
for (j = 1; j < 16; j++)
86938694
{
86948695
if (i + j >= data->dsSize)
86958696
break;
8696-
printf(", %02Xh", *reinterpret_cast<uint8_t*>(&data->dsCont[i + j]));
8697+
printf(", %02Xh", *reinterpret_cast<uint8_t*>(&data->Data()[i + j]));
86978698
}
86988699
i += j;
86998700
break;
@@ -8703,12 +8704,12 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
87038704
{
87048705
printf("\t<Unexpected data size %d (expected size%%2 == 0)\n", data->dsSize);
87058706
}
8706-
printf("\tdw\t%04Xh", *reinterpret_cast<uint16_t*>(&data->dsCont[i]));
8707+
printf("\tdw\t%04Xh", *reinterpret_cast<uint16_t*>(&data->Data()[i]));
87078708
for (j = 2; j < 24; j += 2)
87088709
{
87098710
if (i + j >= data->dsSize)
87108711
break;
8711-
printf(", %04Xh", *reinterpret_cast<uint16_t*>(&data->dsCont[i + j]));
8712+
printf(", %04Xh", *reinterpret_cast<uint16_t*>(&data->Data()[i + j]));
87128713
}
87138714
i += j;
87148715
break;
@@ -8719,12 +8720,12 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
87198720
{
87208721
printf("\t<Unexpected data size %d (expected size%%4 == 0)\n", data->dsSize);
87218722
}
8722-
printf("\tdd\t%08Xh", *reinterpret_cast<uint32_t*>(&data->dsCont[i]));
8723+
printf("\tdd\t%08Xh", *reinterpret_cast<uint32_t*>(&data->Data()[i]));
87238724
for (j = 4; j < 24; j += 4)
87248725
{
87258726
if (i + j >= data->dsSize)
87268727
break;
8727-
printf(", %08Xh", *reinterpret_cast<uint32_t*>(&data->dsCont[i + j]));
8728+
printf(", %08Xh", *reinterpret_cast<uint32_t*>(&data->Data()[i + j]));
87288729
}
87298730
i += j;
87308731
break;
@@ -8737,12 +8738,12 @@ void emitter::emitDispDataSec(dataSecDsc* section, AllocMemChunk* dataChunks)
87378738
{
87388739
printf("\t<Unexpected data size %d (expected size%%8 == 0)\n", data->dsSize);
87398740
}
8740-
printf("\tdq\t%016llXh", *reinterpret_cast<uint64_t*>(&data->dsCont[i]));
8741+
printf("\tdq\t%016llXh", *reinterpret_cast<uint64_t*>(&data->Data()[i]));
87418742
for (j = 8; j < 64; j += 8)
87428743
{
87438744
if (i + j >= data->dsSize)
87448745
break;
8745-
printf(", %016llXh", *reinterpret_cast<uint64_t*>(&data->dsCont[i + j]));
8746+
printf(", %016llXh", *reinterpret_cast<uint64_t*>(&data->Data()[i + j]));
87468747
}
87478748
i += j;
87488749
break;

src/coreclr/jit/emit.h

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3555,10 +3555,32 @@ class emitter
35553555
sectionType dsType;
35563556
var_types dsDataType;
35573557

3558-
// variable-sized array used to store the constant data, BasicBlock*
3559-
// array in the block cases, or emitLocation for the asyncResumeInfo
3560-
// case.
3561-
BYTE dsCont[0];
3558+
private:
3559+
union
3560+
{
3561+
BYTE* dsData; // for data blobs
3562+
BasicBlock** dsBlocks; // for block-based sections
3563+
emitLocation* dsLocations; // for async resume info
3564+
};
3565+
3566+
public:
3567+
BYTE*& Data()
3568+
{
3569+
assert(dsType == sectionType::data);
3570+
return dsData;
3571+
}
3572+
3573+
BasicBlock**& Blocks()
3574+
{
3575+
assert((dsType == sectionType::blockAbsoluteAddr) || (dsType == sectionType::blockRelative32));
3576+
return dsBlocks;
3577+
}
3578+
3579+
emitLocation*& Locations()
3580+
{
3581+
assert(dsType == sectionType::asyncResumeInfo);
3582+
return dsLocations;
3583+
}
35623584
};
35633585

35643586
/* These describe the entire initialized/uninitialized data sections */

src/coreclr/jit/emitarm.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7326,7 +7326,7 @@ void emitter::emitDispInsHelp(
73267326
if (jdsc != nullptr && id->idIns() == INS_movt)
73277327
{
73287328
unsigned cnt = jdsc->dsSize / TARGET_POINTER_SIZE;
7329-
BasicBlock** bbp = (BasicBlock**)jdsc->dsCont;
7329+
BasicBlock** bbp = jdsc->Blocks();
73307330

73317331
bool isBound = (emitCodeGetCookie(*bbp) != nullptr);
73327332

src/coreclr/jit/emitxarch.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12347,7 +12347,7 @@ void emitter::emitDispAddrMode(instrDesc* id, bool noDetail) const
1234712347
if (jdsc && !noDetail)
1234812348
{
1234912349
unsigned cnt = (jdsc->dsSize - 1) / TARGET_POINTER_SIZE;
12350-
BasicBlock** bbp = (BasicBlock**)jdsc->dsCont;
12350+
BasicBlock** bbp = jdsc->Blocks();
1235112351

1235212352
#ifdef TARGET_AMD64
1235312353
#define SIZE_LETTER "Q"

0 commit comments

Comments
 (0)