Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document what the compatibility with the new low-level lift architecture is for each pass #336

Open
frabert opened this issue Nov 17, 2022 · 5 comments

Comments

@frabert
Copy link
Contributor

frabert commented Nov 17, 2022

Status Pass
BranchRecovery
CombineAdjacentShifts
ConvertAddressesToEntityUses
ConvertMasksToCasts
ConvertSymbolicReturnAddressToConcereteReturnAddress
ConvertXorsToCmps
HoistUsersOfSelectsAndPhis
LowerRemillMemoryAccessIntrinsics
LowerSwitchIntrinsics
LowerTypeHintIntrinsics
RecoverBasicStackFrame
RemoveCompilerBarriers
RemoveDelaySlotIntrinsics
RemoveErrorIntrinsics
RemoveRemillFunctionReturns
RemoveStackPointerCExprs
RemoveTrivialPhisAndSelects
RemoveUnusedBranchHints
RemoveUnusedFPClassificationCalls
SinkSelectionsIntoBranchTargets
SplitStackFrameAtReturnAddress
SpreadPCMetadata
TransformRemillJumpIntrinsics
@frabert
Copy link
Contributor Author

frabert commented Nov 18, 2022

Are we even generating type hints anymore?

@frabert
Copy link
Contributor Author

frabert commented Nov 18, 2022

As is, RecoverBasicStackFrame would work, but it would try to recover a different stack frame for each block, which is probably not what we want

@frabert
Copy link
Contributor Author

frabert commented Nov 18, 2022

SinkSelectionsIntoBranchTargets needs work, or at even rethinking if we need what it is trying to do

@2over12
Copy link
Collaborator

2over12 commented Nov 18, 2022

I'll fill some in here LowerSwitchIntrinsics is a fundamentally flawed concept in the low-level lift architecture since it is effectively a control flow restructuring, we probably will just want to get rid of this for now. Maybe it would be cool to use it to allow the user to more conveniently edit edges by manipulating the switch case, but for now, I'd say we don't need it. Like you mentioned type hints aren't generated so don't need that.

ConvertAddressesToEntityUses is critical and we do need it. The tricky bit is if a reference is partially built up in multiple blocks. ie. we get a pointer then do an offset into it in a later block.

RemoveRemillFunctionReturns is a serious open question. How do we want to represent interprocedural flow like this for codegen. changing this to a return in the basic block function doesn't help since that wont look like a return in the parent function. Something tells me we kinda want to keep these around but this may block optimization.

RemoveUnusedBranchHints should work and we should apply it so we don't keep remill branch hints around after branch recovery.

RemoveStackPointerCExprs is part of making RecoverBasicStackFrame work in some cases so if we hold onto that then we hold onto this/maybe the whole strategy will change.

SplitStackFrameAtReturnAddress similar to CExprs it's part of arranging the stack frame I think we need to think about how we are representing the stack now.

SpreadPCMetadata I mean it should work... it's kinda sketchy though and we can probably orient ourselves better based on basic blocks.

TransformRemillJumpIntrinsics I don't think this will work as is. We wont see the return address flow to the block. We probably want something like this though to recover returns so that we can emit an idiomatic return during codegen

Overall I think the action items are:

  • Figure out how we are going to represent and abstract the stack
  • Figure out how we are going to represent interprocedural control flow: returns and calls
  • Figure out how to get the contextual information needed to recover the above + do address->entity conversions

@frabert
Copy link
Contributor Author

frabert commented Nov 18, 2022

Could be make the PC annotations be per block function instead of per instruction? I.e. annotate every block function with the range of PCs it spans

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants