Page MenuHomePhabricator

[RISC-V][HWASAN] Add intrinsics required for HWASAN support for RISC-V
ClosedPublic

Authored by smd on Aug 6 2022, 2:44 PM.

Details

Summary

[2/9] patch series to port HWASAN for riscv64

Depends On D131339

Diff Detail

Event Timeline

smd created this revision.Aug 6 2022, 2:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2022, 2:44 PM
smd added a project: Restricted Project.Aug 6 2022, 3:06 PM
smd published this revision for review.Aug 7 2022, 3:28 AM
craig.topper added inline comments.
llvm/lib/Target/RISCV/RISCVRegisterInfo.td
149 ↗(On Diff #450574)

I think this line probably exceeds 80 characters

smd updated this revision to Diff 450700.Aug 7 2022, 10:24 PM

Addressing comments

smd added inline comments.Aug 7 2022, 10:26 PM
llvm/lib/Target/RISCV/RISCVRegisterInfo.td
149 ↗(On Diff #450574)

Fixed, thanks

vitalybuka accepted this revision.Aug 9 2022, 5:42 PM
This revision is now accepted and ready to land.Aug 9 2022, 5:42 PM
jrtc27 added inline comments.Aug 9 2022, 6:13 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1669–1670

This formatting is not used anywhere in the RISCV backend

llvm/lib/Target/RISCV/RISCVRegisterInfo.td
149 ↗(On Diff #450574)

Why is this set of registers what it is? This is not a helpful name. It looks like this is in fact the set of GPRs minus RA and all temporary registers other than t0. The AArch64 equivalent has:

// Register set that excludes registers that are reserved for procedure calls.
// This is used for pseudo-instructions that are actually implemented using a
// procedure call.
smd updated this revision to Diff 451458.Aug 10 2022, 7:49 AM

Addressing comments

smd added inline comments.Aug 10 2022, 7:56 AM
llvm/lib/Target/RISCV/RISCVRegisterInfo.td
149 ↗(On Diff #450574)

Thanks for catching this, you're right, there's no need to introduce a new register class like this and GPRJALR should be used instead.

smd edited the summary of this revision. (Show Details)Aug 10 2022, 7:56 AM
smd updated this revision to Diff 453083.Aug 16 2022, 11:46 AM

Addressing comments

llvm/lib/Target/RISCV/RISCVInstrInfo.td
1669–1670

Should be fixed now, thanks.

jrtc27 added inline comments.Aug 22 2022, 10:53 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1669–1670

It is not

smd updated this revision to Diff 454955.Aug 23 2022, 2:05 PM

Addressing comments

smd added inline comments.Aug 23 2022, 2:19 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.td
1669–1670

Could you please check this kind of formatting?
If you still find it wrong could you please suggest the proper one, thanks

This revision was landed with ongoing or failed builds.Aug 28 2022, 8:05 AM
This revision was automatically updated to reflect the committed changes.