Skip to content

Conversation

@Garrett-Bodley
Copy link

Adds support for internal assembly functions. The new InternalFunction() call outputs a TEXT directive sans the pesky unicode dot used in Go assembly. This allows Avo to generate an internal assembly function that is not linked to any symbols in the corresponding package.

Closes #442

Adds support for internal assembly functions. The new InternalFunction()
call outputs a TEXT directive sans the pesky unicode dot used in Go
assembly. This allows Avo to generate an internal assembly function that
is not linked to any symbols in the corresponding package.

Closes mmcloughlin#442
@FiloSottile
Copy link

It might be worth making these always static (ending in <> like TEXT p256SqrInternal<>(SB),NOSPLIT,$0) so that they don't conflict with symbols with the same name in other packages or modules.

@Garrett-Bodley
Copy link
Author

There are a number of internal functions in crypto/internal/nistec/p256_asm_amd64.s that lack the static <> characters. I wasn't sure if that was an intentional decision or not.

Making all internal functions static in Avo would result in a diff on those lines when I port that file to Avo, but maybe that is desirable? If those symbols are non-static for a reason then we should leave the PR as is.

I'm fine with either option just wanted to flag that before a decision was made.

If we make all internal functions Static:

Could make sense to make the name field of the Function struct a Symbol instead of the current string

avo/ir/ir.go

Lines 172 to 190 in 13668f4

type Function struct {
Name string
Attributes attr.Attribute
Pragmas []Pragma
Doc []string
Signature *gotypes.Signature
LocalSize int
Nodes []Node
// LabelTarget maps from label name to the following instruction.
LabelTarget map[Label]*Instruction
// Register allocation.
Allocation reg.Allocation
// ISA is the list of required instruction set extensions.
ISA []string
}

The Symbol struct already has a static field and some associated printing logic.

avo/operand/types.go

Lines 14 to 31 in 13668f4

// Symbol represents a symbol name.
type Symbol struct {
Name string
Static bool // only visible in current source file
}
// NewStaticSymbol builds a static Symbol. Static symbols are only visible in the current source file.
func NewStaticSymbol(name string) Symbol {
return Symbol{Name: name, Static: true}
}
func (s Symbol) String() string {
n := s.Name
if s.Static {
n += "<>"
}
return n
}

Happy to add that to this PR

@mmcloughlin do you have thoughts?

Examples of non-static internal functions:

https://github.com/golang/go/blob/27093581b2828a2752a6d2711def09517eb2513b/src/crypto/internal/nistec/p256_asm_amd64.s#L1318
https://github.com/golang/go/blob/27093581b2828a2752a6d2711def09517eb2513b/src/crypto/internal/nistec/p256_asm_amd64.s#L1344
https://github.com/golang/go/blob/27093581b2828a2752a6d2711def09517eb2513b/src/crypto/internal/nistec/p256_asm_amd64.s#L1527
https://github.com/golang/go/blob/27093581b2828a2752a6d2711def09517eb2513b/src/crypto/internal/nistec/p256_asm_amd64.s#L1996

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.

build,ir, printer: Support for internal assembly functions

2 participants