reject non-pointer type for global

[dannypsnl]

If we define a new global

module.NewGlobal(name, types.I8)

it would cause an error that global variable reference must have pointer type

Thus, I think this check is acceptable.

[mewmew]

Definitely!

This should be part of an irsem package, that is yet to be written :) Along with other semantic analysis validation and error checking.

Cheers,
Robin

[mewmew]

Just to clarify, what tool is reporting the error it would cause an error that global variable reference must have pointer type? Is this the LLVM compiler?

[dannypsnl]

yes

[dannypsnl]

BTW, do you think https://github.com/dannypsnl/extend is a good way to do irsem?

[mewmew]

BTW, do you think https://github.com/dannypsnl/extend is a good way to do irsem?

I consider dannypsnl/extend to be closer to irutil/irgen.

For irsem we would do a traversal over an entire module, validating it's semantics, e.g. by calling irsem.Verify.

ref: #65 (comment)

// Package sem performs semantic analysis of LLVM IR modules.
package sem

// Verify performs type-checking and semantic analysis of the given module to
// verify consistency and correctness.
func Verify(m *ir.Module) error

Edit: for inspiration, here is a very primitive semantic analysis written for the uC programming language by @sangisos and me. uc/sem. It essentially traverses the AST tree, checking that various invariants hold, that the type checking gives no type errors, and that the semantics are not broken in obvious ways.

[dannypsnl]

https://github.com/llir/irsem solution

[mewmew]

re: https://github.com/llir/irsem/blob/cda51dc46b55de911196e57f5848c5c828edf1a9/verify.go#L11

@dannypsnl, are you sure this semantic is not valid in LLVM IR? I think it is.

package main

import (
	"fmt"
	"log"

	"github.com/llir/llvm/ir"
	"github.com/llir/llvm/ir/constant"
	"github.com/llir/llvm/ir/types"
)

func main() {
	if err := foo(); err != nil {
		log.Fatalf("%+v", err)
	}
}

func foo() error {
	m := ir.NewModule()
	x := m.NewGlobalDef("x", constant.NewInt(types.I32, 42))
	f := m.NewFunc("main", types.I32)
	entry := f.NewBlock("entry")
	result := entry.NewLoad(types.I32, x)
	entry.NewRet(result)
	fmt.Println(m)
	log.Println("x.ContentType:", x.ContentType)
	log.Println("x.Type():", x.Type())
	return nil
}

Outputs the debug message:

x.ContentType: i32
x.Type(): i32*

And the LLVM IR:

@x = global i32 42

define i32 @main() {
entry:
	%0 = load i32, i32* @x
	ret i32 %0
}

The output LLVM IR is valid when run through lli

# lli a.ll ; echo $?
42

As such, the type of *ir.Global.ContentType may be a non-pointer type, and this is still valid. I think the only thing is we have to reference global variables using pointer types, that is, when you load from a global variable, or store to a global variable, then the global has pointer type (and not it's content type).

Could you provide a full test case/example which demonstrates the broken behaviour?

Cheers,
Robin

[dannypsnl]

mod := ir.NewModule()
mod.NewGlobal("hello", types.I8)
mod.NewGlobalDef("x", constant.NewInt(types.I32, 42))
mf := mod.NewFunc("main", types.I32)
mb := mf.NewBlock("")
mb.NewRet(constant.NewInt(types.I32, 0))

write to temporary llvm file

@hello = global i8
@x = global i32 42

define i32 @main() {
0:
	ret i32 0
}

compile

llc: error: llc: tmp.ll:2:1: error: global variable reference must have pointer type
@x = global i32 42
^

[dannypsnl]

But have to notice, if we only write

mod.NewGlobal("hello", types.I8)

compile produces

llc: error: llc: tmp.ll:3:1: error: expected value token

so this seems more like an inconsistent error report from llc

[mewmew]

Ah, it's missing external.

-@hello = global i8
+@hello = external global i8
 @x = global i32 42
 
 define i32 @main() {
 0:
 	ret i32 0
 }

Therefore, the @x is read as the initializing value of @hello.

[dannypsnl]

Ah, it's missing external.

-@hello = global i8
+@hello = external global i8
 @x = global i32 42
 
 define i32 @main() {
 0:
 	ret i32 0
 }

Therefore, the @x is read as the initializing value of @hello.

Emmm, should we fix this automatically? Or check in irsem

I think the check would be a global missing initializer should with external

[mewmew]

Emmm, should we fix this automatically? Or check in irsem

The check can be added to irsem. If global.Init == nil and global.Linkage is the zero value (LinkageNone), or something along those lines.

[dannypsnl]

Then please check 30da92f3d2381f91fd7af9edfbb5633b49a647d0, if ok then I close this issue.

[mewmew]

LGTM, with minor comment llir/irsem@30da92f#r62298445