Skip to content

Conversation

@vext01
Copy link
Contributor

@vext01 vext01 commented May 21, 2025

}
}

pub(crate) fn decopy(&self, m: &Module) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a docstring. I think "If self is Operand::Var and references a Copy, recursively search until a non-Copy instruction is found." or similar will do.


let func = self.aot_mod.func(*callee);
if inst.is_mappable_call(self.aot_mod)
let mappable = !func.is_declaration();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable feels a bit weird and isn't used anywhere except the next line? Maybe just inline it into the if so it's if !func.is_declaration()?

@ltratt
Copy link
Contributor

ltratt commented May 21, 2025

Two minor comments which can be force pushed.

@vext01
Copy link
Contributor Author

vext01 commented May 21, 2025

Force pushed fixes.

Also bumped the yklua CI version and formatted.

Don't forget to merge ykjit/yklua#125 first.

@vext01
Copy link
Contributor Author

vext01 commented May 21, 2025

Force pushed again with a fixed yklua.

@ltratt ltratt added this pull request to the merge queue May 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 21, 2025
@ltratt ltratt added this pull request to the merge queue May 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025
@vext01
Copy link
Contributor Author

vext01 commented May 23, 2025

resynced ykllvm, which ought to fix this.

requires: ykjit/ykllvm#260

@ptersilie ptersilie added this pull request to the merge queue May 23, 2025
@ptersilie ptersilie removed this pull request from the merge queue due to a manual request May 23, 2025
@ptersilie ptersilie added this pull request to the merge queue May 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025
@ptersilie
Copy link
Contributor

ld.lld: /ci/ykllvm/llvm/lib/YkIR/YkIRWriter.cpp:608: void (anonymous namespace)::YkIRWriter::serialiseStackmapCall(llvm::CallInst *, (anonymous namespace)::FuncLowerCtxt &): Assertion `I->getCalledFunction()->isIntrinsic()' failed.

@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

Force pushed test fixes. Note that ykjit/ykllvm#261 is required first.

@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

@ptersilie I think this is ready to go now Laurie's change finished benchmarking.

@ptersilie ptersilie added this pull request to the merge queue May 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2025
@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

Once ykjit/ykllvm#262 is in, I sincerely hope that this will merge once and for all!

It will need squashing -- ok for me to do that?

@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

@ptersilie perhaps you can approve squashing in Laurie's absence.

There's only a merge commit and a ykllvm resync since the last review.

@ltratt
Copy link
Contributor

ltratt commented May 27, 2025

Please squash.

vext01 added 2 commits May 27, 2025 16:06
These show up when you start to use C function pointers.

There were two options:
 - introduce a `jit_ir::Const::Func`
 - convert AOT function operands to constant pointers.

This is the latter.
@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

squashed and ready to merge.

@ltratt ltratt added this pull request to the merge queue May 27, 2025
github-merge-queue bot pushed a commit that referenced this pull request May 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 27, 2025
In turn this allows us, in some scenarios, to inline indirect calls into
the trace.
@vext01
Copy link
Contributor Author

vext01 commented May 27, 2025

Force pushed swt fixes.

@ptersilie ptersilie added this pull request to the merge queue May 27, 2025
Merged via the queue into ykjit:master with commit b2a8d14 May 27, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants