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

Internal value caching doesn't track RAUW and deletion #43

Closed
smaillet opened this issue Dec 23, 2017 · 0 comments
Closed

Internal value caching doesn't track RAUW and deletion #43

smaillet opened this issue Dec 23, 2017 · 0 comments

Comments

@smaillet
Copy link
Member

Found while testing the full integration and support of OrcJit:

Repro:
Run Kaleidoscope 6 or 7 with the sample Kaleidoscope code from the LLVM Tutorial.

# Logical unary not.
def unary!(v)
  if v then
    0
  else
    1;

# Unary negate.
def unary-(v)
  0-v;

# Define > with the same precedence as <.
def binary> 10 (LHS RHS)
  RHS < LHS;

# Binary logical or, which does not short circuit.
def binary| 5 (LHS RHS)
  if LHS then
    1
  else if RHS then
    1
  else
    0;

# Binary logical and, which does not short circuit.
def binary& 6 (LHS RHS)
  if !LHS then
    0
  else
    !!RHS;

# Define = with slightly lower precedence than relationals.
def binary = 9 (LHS RHS)
  !(LHS < RHS | LHS > RHS);

# Define ':' for sequencing: as a low-precedence operator that ignores operands
# and just returns the RHS.
def binary : 1 (x y) y;

Root Cause:
The internal value caching doesn't track Replace All Uses With and deletion operations for Values. If the context is shared by a number of modules that come and go or the functions within them are transformed, then it is highly likely that any number of them will become invalid because some transform replaced or removed an instruction etc.. This ends up with a dangling pointer from managed code side. This becomes a blatant fail when LLVM has re-used the memory for some other type, which may not be a Value at all causing a crash, or if it is a value an InvalidCastException because the resolving to a specific wrapped type doesn't match the new reality.

smaillet added a commit to smaillet/Llvm.NET that referenced this issue Dec 27, 2017
* Kaleidoscope now correctly runs the mandelbrot example!
* Completely re-wrote Value handle interning/cache to use a tracking map that handles RAUW and deletions from various transforms.
* Updated Kaleidosopce samples to use OrcJit
* Fixed  KaleidoscopeJIT and generator to handle re-defining a function by tracking the jitmodule handles for a function and removing the module for the previous definition before adding the new one.
* Added support for global symbol resolving to delegates so that JITed code can call back to the host in a controlled fashion.
* Fixed bug where user defined operator expressions were re-generating the operand expressions.
* Added DGML generator for the Kaleidoscope parser to make visualizing the parse tree a LOT easier.
* Refactored common support and JIT for Kaledoscope to eliminate most of the duplicated code files.
* refactored Kaleidoscope grammar to split out the individual context rules
* re-worked dynamic operator precedence to fix incorrect evaluations
* reorganized the various codegnerator.cs files to make doing a diff between tutorial chapters easier to see what's new for each chapter.
* Fixed bug in for loop code generation where condition was inverted so it basically guaranteed only a single loop iteration always.
* Fixed bug in OrcJit where the symbolresolver delegate was at the mercy of the garbage collector, It is now contained in a WrappedNativeCallback that is mapped per module.
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

1 participant