Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Codegen: inline constants #4904

Merged
merged 1 commit into from
Aug 31, 2017
Merged

Codegen: inline constants #4904

merged 1 commit into from
Aug 31, 2017
+108 −4

Conversation

asterite
Copy link
Member

Fixes #4884

This PR changes the codegen in two ways:

  1. Constants with simple literals are no longer lazily initialized. This includes bool, number, chars and strings literals.
  2. Integer constants, such as literals, or integers that can be computed at compile-time, are now always inlined.

Ideally point 2 should work for float constants too, but right now it doesn't because the math interpreter code only targets integers and it will be a much bigger refactor which can be tackled in a separate PR. Also, float constants are usually less common than integer ones.

So for example, compiling with --prelude=empty --emit llvm-ir --no-debug, for this code:

A = 1
A

Before:

@A = internal constant i32 1
@"A:init" = internal global i1 false

define internal i32 @__crystal_main(i32 %argc, i8** %argv) {
entry:
  store i32 %argc, i32* @ARGC_UNSAFE
  store i8** %argv, i8*** @ARGV_UNSAFE
  %0 = call i32* @"~A:read"()
  %1 = load i32, i32* %0
  ret i32 %1
}

define internal i32* @"~A:read"() {
entry:
  %0 = load i1, i1* @"A:init"
  br i1 %0, label %initialized, label %not_initialized

initialized:                                      ; preds = %not_initialized, %entry
  ret i32* @A

not_initialized:                                  ; preds = %entry
  store i1 true, i1* @"A:init"
  call void @"~A:init"()
  br label %initialized
}

define internal void @"~A:init"() {
entry:
  ret void
}

; more code omitted

After:

define internal i32 @__crystal_main(i32 %argc, i8** %argv) {
entry:
  store i32 %argc, i32* @ARGC_UNSAFE
  store i8** %argv, i8*** @ARGV_UNSAFE
  ret i32 1
}

Likewise, for this:

B = 1 + 2
A = B * 3
A

Before it was a long chunk of LLVM code, now:

define internal i32 @__crystal_main(i32 %argc, i8** %argv) {
entry:
  store i32 %argc, i32* @ARGC_UNSAFE
  store i8** %argv, i8*** @ARGV_UNSAFE
  ret i32 9
}

Note that this is without --release. That means it generates less code and supposedly will improve LLVM optimization times.

@RX14
Copy link
Member

RX14 commented Aug 30, 2017

Where in this code is the change regarding string literals?

asterite
asterite commented Aug 31, 2017
View reviewed changes
src/compiler/crystal/codegen/const.cr
end

def read_const_pointer(const)
if const == @program.argc || const == @program.argv
if const == @program.argc || const == @program.argv || const.initializer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RX14 Here: if const.initializeris set it means the constant could be computed at compile-time. The assignment to initializer happens somewhere in this file too (const.cr).

As usual, the code is pretty messy... :/

@RX14
Copy link
Member

RX14 commented Aug 31, 2017

I'm a little concerned about debugability when inlining in non-release mode. But the code itself looks great to me.

@asterite
Copy link
Member Author

@RX14 I think debugability is not an issue because inlined values have nothing that can be inspected or stepped-into: they are just constants, or simple math operations.

@RX14 RX14 merged commit 435d6e1 into crystal-lang:master Aug 31, 2017
@asterite asterite deleted the feature/inline-consts branch August 31, 2017 13:38
@RX14 RX14 added this to the Next milestone Aug 31, 2017
@akzhan akzhan mentioned this pull request Sep 1, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RX14 RX14

Assignees
No one assigned
Labels
kind:feature performance topic:compiler
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@asterite @RX14