Skip to content

Commit 66d94c6

Browse files
committed
Add 2 FIXME comments
1 parent 2e22a1d commit 66d94c6

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

bolt/lib/Passes/NonPacProtectedRetAnalysis.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,16 @@ class PacRetAnalysis
261261
if (BC.MIB->isCall(Point))
262262
Written.set();
263263
else
264+
// FIXME: `getWrittenRegs` only sets the register directly written in the
265+
// instruction, and the smaller aliasing registers. It does not set the
266+
// larger aliasing registers. To also set the larger aliasing registers,
267+
// we'd have to call `getClobberedRegs`.
268+
// It is unclear if there is any test case which shows a different
269+
// behaviour between using `getWrittenRegs` vs `getClobberedRegs`. We'd
270+
// first would like to see such a test case before making a decision
271+
// on whether using `getClobberedRegs` below would be better.
272+
// Also see the discussion on this at
273+
// https://github.com/llvm/llvm-project/pull/122304#discussion_r1939511909
264274
BC.MIB->getWrittenRegs(Point, Written);
265275
Next.NonAutClobRegs |= Written;
266276
// Keep track of this instruction if it writes to any of the registers we
@@ -271,6 +281,10 @@ class PacRetAnalysis
271281

272282
ErrorOr<MCPhysReg> AutReg = BC.MIB->getAuthenticatedReg(Point);
273283
if (AutReg && *AutReg != BC.MIB->getNoRegister()) {
284+
// FIXME: should we use `OnlySmaller=false` below? See similar
285+
// FIXME about `getWrittenRegs` above and further discussion about this
286+
// at
287+
// https://github.com/llvm/llvm-project/pull/122304#discussion_r1939515516
274288
Next.NonAutClobRegs.reset(
275289
BC.MIB->getAliases(*AutReg, /*OnlySmaller=*/true));
276290
if (TrackingLastInsts && isTrackingReg(*AutReg))

0 commit comments

Comments
 (0)