[PATCH] D131347: Set up basic infrastructure for ELF/i386 backend support in JITLink .

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

I ran out of time to test this today, but should be able to commit early tomorrow.



================
Comment at: llvm/test/ExecutionEngine/JITLink/i386/ELF_i386_minimal.s:3
+# RUN: llvm-mc -triple=i386-unknown-linux-gnu -position-independent -filetype=obj -o %t/i386_simple.o %s
+# RUN: llvm-jitlink %t/i386_simple.o; result=$?; [ $result -eq 42 ] || exit 1
+
----------------
jain98 wrote:
> lhames wrote:
> > lhames wrote:
> > > jain98 wrote:
> > > > jain98 wrote:
> > > > > lhames wrote:
> > > > > > Tests should be `-noexec` (except in special circumstances): We want them to be able to run on architectures other than the target.
> > > > > > 
> > > > > > I would simplify to:
> > > > > > ```
> > > > > > # RUN: llvm-mc -triple=i386-unknown-linux-gnu -position-independent -filetype=obj -o %t.o %s
> > > > > > # RUN: llvm-jitlink -noexec %t.o
> > > > > > ```
> > > > > Does this also mean that the contents of this directory's `lit.local.cfg` should be changed? Currently this is what's in there - 
> > > > > 
> > > > > ```
> > > > > if not 'i386' in config.root.targets:
> > > > >   config.unsupported = True
> > > > > ```
> > > > > 
> > > > > I may be wrong, but I //think// it's saying run this test only if the current host architecture is i386?
> > > > > 
> > > > Additionally, what the reasoning behind making these tests `--noexec` - it looks like most JitLink tests are? I'm wondering what makes the code in these tests non-runnable if we're able to load it into memory? I can see how the generated code for one architecture might be straight up rubbish for some other architecture, and therefore it does not make sense to actually try and execute the generated code. But then why go to the point where we're loading things into memory even? Why not stop after generating the `LinkGraph`?
> > > > I may be wrong, but I think it's saying run this test only if the current host architecture is i386?
> > > 
> > > That check is saying "if this LLVM build isn't configured with i386 backend support". You want to keep it -- it tells us whether or not the tools can handle i386 assembly. :) 
> > > I can see how the generated code for one architecture might be straight up rubbish for some other architecture, and therefore it does not make sense to actually try and execute the generated code.
> > 
> > Yep -- we want to run this on all architectures, and that means not executing it when target arch != host arch. We just go one step further and never execute it at all.
> > 
> > > But then why go to the point where we're loading things into memory even? Why not stop after generating the LinkGraph?
> > 
> > We want to check that all relocations have been applied correctly. Originally this testing system was designed for RuntimeDyld, which had no exposed data structure that we could evaluate, hence the decision to inspect final linked memory instead. Now that we have JITLink we could inspect the graph, but the advantage of checking the final linked memory is that it verifies that linking worked end-to-end, including layout in target memory.
> > 
> > It wouldn't hurt to rethink all this test infrastructure. It was written in a hurry back in 2015 and barely touched since. That said it has been remarkably effective -- a cleanup and new functionality would be welcome (LinkGraph inspection? Maybe a more user-friendly expression language and robust parser), but we'd definitely want to keep some variation of the end-to-end scheme that we have.
> > > But then why go to the point where we're loading things into memory even? Why not stop after generating the LinkGraph?
> > 
> > We want to check that all relocations have been applied correctly. Originally this testing system was designed for RuntimeDyld, which had no exposed data structure that we could evaluate, hence the decision to inspect final linked memory instead. Now that we have JITLink we could inspect the graph, but the advantage of checking the final linked memory is that it verifies that linking worked end-to-end, including layout in target memory.
> 
> Perhaps this is my lack of understanding/knowledge, but are we truly checking whether the relocations have been applied correctly? I'm thinking of a very simple `Pointer32` type relocation for a non-static global var. Is it possible that we calculate the address of the variable to be relocated incorrectly and since we never run the program we never run into any issues? 
> 
> I'm guessing perhaps the more concrete issues, such as, invalid memory access are checked for, but can the program be linked and still be logically incorrect?
> Is it possible that we calculate the address of the variable to be relocated incorrectly and since we never run the program we never run into any issues?

It is, but the address plumbing is shared infrastructure used by all backends and implicitly tested by the execution tests elsewhere in `llvm/test/ExecutionEngine`, so we don't need to re-test it in the JITLink backend tests.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131347/new/

https://reviews.llvm.org/D131347