What does the LLVM patch do?

[Gulshan Singh]

Out of curiosity (and a bit of annoyance), I'm wondering why LLVM has to be patched for extempore to work? For reference, here's the patch: https://github.com/digego/extempore/blob/master/extras/llparser.patch

The change is fairly small, but I don't really know what's being changed. I'd like to understand the patch just so there's some documentation online as to why we have to do this, and to make sure there isn't some workaround that could be used instead.

[Ben Swift]

Hi Gulshan

It still is necessary, unfortunately. I'll leave the explanation of why up to
Andy :)

Cheers,
Ben

[Ricardo Wurmus]

Hi,

I would also like to know why the patch is needed. Has it been reported
to LLVM upstream? I just patched Extempore to build with LLVM 3.5 but
the patch still seems to be necessary.

Even in LLVM 3.6 this area of LLParser.cpp has not been touched, so I
wonder if this issue is a) at all known to LLVM upstream, and b) really
a problem that should be fixed in LLVM or rather an issue in Extempore.

The comment says

// If the type hasn't been defined yet, create a forward definition and
// remember where that forward def'n was seen (in case it never is defined).

Not knowing anything about LLVM or Extempore it seems to me that the
patch would not be needed if the encountered type with name
‘Lex.getStrVal()’ would only need to be a member of ‘NamedTypes’.

Looking at the patch I wonder why ‘M->getTypeByName(Lex.getStrVal())’
would return a type when ‘NamedTypes[Lex.getStrVal()]’ does not.
‘ParseStructDefinition’ should take care of recording the encountered
type in ‘NamedTypes’ already.

I think this problem should be solved in Extempore rather than in LLVM.
It would help to know what the actual problem is this patch should fix.

The error when loading “libs/core/std.xtm” is:

SetValue: NaNf >>> float
SetValue: NaNd >>> double
Extempore: <string>:46:18: error: '@llvm_zone_malloc' defined with type 'i8* (%mzone*, i64)*'
%dat9 = call i8* @llvm_zone_malloc(%mzone* %zone8, i64 4)

I don’t understand the error; does it complain about a type mismatch?

~~ Ricardo

[Ben Swift]

Hi Ricardo

The LLVM patch is from back in the day (Extempore originally built
against LLVM 2). I think @digego did some work at the time to try and
get it upstreamed (with no success), but that was 5+ years ago so I
can't remember the details. Since then, it's become part of the
furniture.

> I would also like to know why the patch is needed. Has it been reported
> to LLVM upstream? I just patched Extempore to build with LLVM 3.5 but
> the patch still seems to be necessary.

> Even in LLVM 3.6 this area of LLParser.cpp has not been touched, so I
> wonder if this issue is a) at all known to LLVM upstream, and b) really
> a problem that should be fixed in LLVM or rather an issue in Extempore.

> The comment says
>
> // If the type hasn't been defined yet, create a forward definition and
> // remember where that forward def'n was seen (in case it never is defined).
>
> Not knowing anything about LLVM or Extempore it seems to me that the
> patch would not be needed if the encountered type with name
> 'Lex.getStrVal()' would only need to be a member of 'NamedTypes'.
>
> Looking at the patch I wonder why 'M->getTypeByName(Lex.getStrVal())'
> would return a type when 'NamedTypes[Lex.getStrVal()]' does not.
> 'ParseStructDefinition' should take care of recording the encountered
> type in 'NamedTypes' already.

Thanks for investigating - I'll have a look.

> I think this problem should be solved in Extempore rather than in LLVM.
> It would help to know what the actual problem is this patch should fix.

If possible, then that would certainly be nice. As the primary
"packager" of Extempore (a job which I do well in some ways and poorly
in others) then I'd love to not have to maintain my own fork of the LLVM
packages on each system. Homebrew has helped with this so far, but I
agree it's an impediment to other packaging options.

> The error when loading "libs/core/std.xtm" is:
>
> SetValue: NaNf >>> float
> SetValue: NaNd >>> double
> Extempore: <string>:46:18: error: '@llvm_zone_malloc' defined with type 'i8* (%mzone*, i64)*'
> %dat9 = call i8* @llvm_zone_malloc(%mzone* %zone8, i64 4)
>
> I don't understand the error; does it complain about a type mismatch?

The error is actually from LLVM, it's saying that Extempore has
generated the LLVM IR, but that LLVM can't compile it (even though it
should be able to, and can after the patch is applied).

Thanks for the heads up on this (and see my comments on the GH issues
you opened for more info). I'll see what I can find out.

[Ricardo Wurmus]

Hi Ben,

thanks for taking the time to respond.

>> The error when loading "libs/core/std.xtm" is:
>>
>> SetValue: NaNf >>> float
>> SetValue: NaNd >>> double
>> Extempore: <string>:46:18: error: '@llvm_zone_malloc' defined with type 'i8* (%mzone*, i64)*'
>> %dat9 = call i8* @llvm_zone_malloc(%mzone* %zone8, i64 4)
>>
>> I don't understand the error; does it complain about a type mismatch?
>
> The error is actually from LLVM, it's saying that Extempore has
> generated the LLVM IR, but that LLVM can't compile it (even though it
> should be able to, and can after the patch is applied).

FWIW, I did compile a patched version of LLVM 3.5, but even when using
that patched version I get this very same error. I have not yet tried
a patched LLVM 3.4, but I may do so eventually.

I find this error puzzling because the line it complains about looks
totally okay to me. If I understand correctly, Extempore generated the
line

%dat9 = call i8* @llvm_zone_malloc(%mzone* %zone8, i64 4)

which is a call to @llvm_zone_malloc returning a value of type ‘pointer
to i8’, passing the values ‘%zone8’ of type ‘pointer to %mzone*’ and ‘4’
of type ‘i64’. This appears to match the type of function
‘@llvm_zone_malloc’, but LLVM seems to disagree.

(I could be totally wrong as I only read the LLVM docs yesterday.)

> Thanks for the heads up on this (and see my comments on the GH issues
> you opened for more info). I'll see what I can find out.

Thanks, I’ll be working on the ‘dev’ branch instead of the latest
release.

[Gulshan Singh]

> I find this error puzzling because the line it complains about looks totally okay to me.

Yup, I thought the same thing when I first saw it as well. I want to view the entire generated LLVM for std.xtm so I can investigate further, is there any way to modify sys:precomp:compile-xtm-file or sys:load to generate the full LLVM file?

The strange thing about the patch code is that the original code seems to be defining a type that it hasn't been seen before, but the patch code has an if statement that checks if the type has already been defined in the module.

I wonder if this is in any way related to the fact that we're calling C code from xtlang code? i.e., the fact that we declare the function prototype here, define the C function somewhere else, and the call that LLVM/C function from xtlang? Just a wild guess, I've called C functions directly from LLVM compiled down to a native binary, but I've never done this type of C/LLVM FFI.

[Andrew Sorensen]

There is an assumption in the parser that all definitions occur within the same compilation unit - i.e. the parser has local state about what has been parsed in this unit of work.  Extempore obviously does lots of little units rather than one big unit and this causes problems for named types that were defined in another unit - which they always are.  The patch simply checks the current module to see if the type has been previously defined, and intervenes appropriately if it has.

I'm not totally sure why this problem doesn't hit more people, although my suspicion is that (a) most people are using much larger compilation units and (b) most people are not using the IR parser directly like we are (i.e. they use the CPP interface primarily).

I asked about this on the llvm mailing list many years ago but did not get any responses.  Happy for someone else to try to push it up again, or perhaps there is a better solution.

Incidently if you want to see IR the easiest way to do this is to define:

(define *impc:compiler:print* #t)

set back to #f at anytime to turn off.

Hope this all makes sense.

Cheers,

Andrew.

--
You received this message because you are subscribed to the Google Groups "Extempore" group.

To unsubscribe from this group and stop receiving emails from it, send an email to extemporelan...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.