search

lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.26k stars 142 forks source link

Add attributes for memory intrinsics #555

Closed tathanhdinh closed 2 years ago

tathanhdinh commented 2 years ago

Hello all,

This is not a ready PR (does not compile yet with LLVM below 3.9, an only for a single 64-bit read intrinsic), just for discussion at this issue.

I still do not know the interference between readnone, set by FindPureIntrinsic for dead store elimination:

https://github.com/lifting-bits/remill/blob/e737070f4827368b49879695bf375d9885e9a2b7/lib/BC/IntrinsicTable.cpp#L58-L60

and readonly and argmemonly.

Many thanks for any review.

pgoodman commented 2 years ago

@tathanhdinh we've stopped supporting llvm versions less than 12, and we plan to move to llvm 13 soon. Do you rely on older versions?

pgoodman commented 2 years ago

Also, for the read-modify-write intrinsics, e.g. cmpxchg, fetch_add, etc. perhaps a __restrict keyword could be used as well.

tathanhdinh commented 2 years ago

@tathanhdinh we've stopped supporting llvm versions less than 12, and we plan to move to llvm 13 soon. Do you rely on older versions?

@pgoodman Ah, I don't know that. I did not follow the development of remill for so long (nearly 2 years!?). So it would compile just fine, I think. A minor remark, I still see:

https://github.com/lifting-bits/remill/blob/e737070f4827368b49879695bf375d9885e9a2b7/include/remill/BC/Version.h#L32-L34

Also, for the read-modify-write intrinsics, e.g. cmpxchg, fetch_add, etc. perhaps a __restrict keyword could be used as well.

Sure, that's just because I still not sure what I do is correct or not (from your response in the issue).

Thank you for the feedback.

pgoodman commented 2 years ago

So right now we mark the intrinsics as readnone, i.e. they don't read or write to memory. So marking them as readonly is perhaps not desirable, but then marking their Memory * as readnone could be. For the read-modify-write intrinsics, we don't mark those as readnone, because they take in an argument by reference (in the code) and pointer (in bitcode). That argument is the value to store and update, and so that value gets read and written, but we could still mark the memory pointer argument as readnone.