Process ptradd instructions and global operands.

[ptersilie]

Add processing of AOT PtrAdd instructions. Since the JIT PtrAdd instruction is limited to 32 bit offsets, do the conversion here and return an error in case of a failure so we can abort the trace.

While we are here, handle global operands by emitting a LoadGlobal instruction and replacing the global operand with an instruction operand referencing the just emitted instruction.

I'm assigning @vext01 too, since there's a bit of a design question here in regards to globals. In previous PRs we decided to separate globals from constants, the way LLVM does, where a global is a subclass of constant. The idea was that this makes it easier in the codegen to know what we are dealing with. However, we couldn't add another Operand type due to space constraints. We thought that wasn't an issue since there's only two instruction using globals, load and store, and so made special versions for them. We thought wrong, as globals can be arguments to functions for example.

I've solved this now by emitting a LoadGlobal instruction whenever we see a global operand. So for example and AOT instruction like

call fprintf(Global(stderr), Global(str))

would be turned into (inside the JIT IR)

$1 = LoadGlobal(stderr)
$2 = LoadGlobal(str)
call fprintf($1, $2)

Whether this is a good idea or not isn't quite clear to me as I don't yet know how globals will be codegenned. All we got is a string and likely have to do some sort of lookup (dlsym?), in which case doing the above should be fine. But there's also a good argument here for why globals should have maybe remained constants. Thoughts?

[ltratt]

But there's also a good argument here for why globals should have maybe remained constants. Thoughts?

My initial reaction is that this should be fine and, indeed, makes our lives simpler. Maybe we can get more clever about this later.

[vext01]

Overall approach seems good to me. At least, I can't say its obviously wrong.

[vext01]

How comes this goes?

[ptersilie]

When we emit the instruction manually from a global (which currently doesn't have a type), what should we put here? I believe the return type of all globals is ptr anyway, so there's probably no point. We do need to somehow work out the actual type later on when codegenning this, but I believe that can be retrieved from the use site. E.g. when we pass the ptr into a call, the call arg will tell us what type it expects.

[vext01]

I believe the return type of all globals is ptr anyway

That depends on the semantics of your load.

In LLVM you'd always get the type that's pointed to.

Let's roll with what you have and see what happens later.

[vext01]

(I wonder if "load global" is a load at all?)

[ptersilie]

I assumed loading a global returns the ptr stored in the global, not the data that ptr points to. But I might be wrong. It's hard to say without looking at examples I guess.

Agreed, let's deal with it when we need to codegen to know exactly what we are dealing with.

[ptersilie]

(I wonder if "load global" is a load at all?)

Yes, I think in our case loading a global is more akin to a lookup where a global name maps to some pointer.

[vext01]

In the meantime, here's an example of how LLVM does it:
https://llvm.org/docs/LangRef.html#id210

i.e. a load is a bit like a deref.

[vext01]

Yes, I think in our case loading a global is more akin to a lookup where a global name maps to some pointer.

Yes.

Maye later (or now, whenever) we should rename the LoadGlobal to something like GlobalAddr?

[ptersilie]

No preference. Though maybe waiting until we know for sure how globals need to be codegenned isn't a bad idea.

[vext01]

Oops :)

[vext01]

Is there a compiler change?

[ptersilie]

Is there a compiler change?

I think I started this before the compiler was merged. So I admit I haven't looked at the compiler yet. You can probably answer that question better than me. 😛

[vext01]

You can probably answer that question better than me.

Looking at the git history, I see that we already serialise ptradd and globals, so I think we are good.

[ptersilie]

14:35:12  --> ykaddr/src/addr.rs:5:12
14:35:12   |
14:35:12 5 | use libc::{self, c_void, dlsym, Dl_info, RTLD_DEFAULT};
14:35:12   |            ^^^^ the item `libc` is already defined here
14:35:12   |
14:35:12   = note: `#[warn(unused_imports)]` on by default
14:35:12 
14:35:12 warning: the item `From` is imported redundantly

I don't quite understand what's going on here, but I hope pulling in master will somehow make this go away.

I also fixed a failing test, which I'm not sure how it slipped through before.

[ltratt]

@ptersilie You can use tryci to test this if you want. Or (and this generally works for me) just run the last command you can see in CI (which in this case is presumably one that checks for warnings and errors if any are found).