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.

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

Brain tape viewer #44

Merged
merged 8 commits into from
Jul 31, 2017
Merged

Brain tape viewer #44

merged 8 commits into from
Jul 31, 2017

Conversation

haskellcamargo
Copy link
Member

@haskellcamargo haskellcamargo commented Jul 11, 2017

This pull-request implements a natural tape vision for the Brain compiler using ASCII only and fixes the version of LLVM and Clang to 3.8 because it was not explicit and the system, by default, uses the version as suffix for operations.

About the tape, we are currently overriding the debug # to show the information, but this can be discussed.

Screenshot

Brain tape

Observation

  • The tests may break because the debug system has changed in overall
  • The code may be optimized. I'm not a C programmer at all

Current work

  • Dynamic size of ^ indicator according to cell value
  • Support for negative numbers
  • Showing 0-value cells as _, except when the index is over it (showing 0)

Future work

  • Ellipsize tape and limit values (on left and right)
  • Alpha representation when possible

To Discuss

  • Use on debug or create a flag -emit-tape

@luizperes
Copy link
Member

luizperes commented Jul 11, 2017

I found a bug on Travis with your PR hahahha! The tests should have not passed!

@luizperes luizperes mentioned this pull request Jul 11, 2017
@luizperes
Copy link
Member

Thank you so much for you PR @haskellcamargo. However I believe that we should fix the Makefile in a better way than that.

Our current problems are:

  • aliases on Linux seem to not be working with clang on Brain.
  • Brain can be compiled with the versions of LLVM 3.5, 3.8 and 3.9 (we have not tested versions 3.6 and 3.7)
  • We have way too many bash scripts. Should we reduce it?

I will open an issue for that as well.

Other than that, could you review the C code @rafaelcn? What do you think of this PR?

@rafaelcn
Copy link
Collaborator

rafaelcn commented Jul 17, 2017

This PR has an amazing idea!

Lets address some issues that we (@luizperes and @rafaelcn) have been discussing @haskellcamargo. The makefile point is exactly what we are having problems. Aliasing clang/llvm has shown to not work as @luizperes previously mentioned and we have to maintain a certain compatibility with older versions of clang/llvm.

Secondly, this debug tape has a bug. When you move more than twelve cells forward or even one cell backwards you won't display the cell correctly. It would be nice to have a way to walk towards the value being shown on the debug on the tape. I'm not sure if this is the intended behavior, but it does have room for improvement!

Another thing that I realized is: we never spoke about the maximum size of a tape in Brain, theoretically it is unlimited but, the limit is actually in the hardware, and as far as the current code is being developed we've been only using ints instead of long ints, this is a problem if the user decides to use a tape longer than 4294967296 (I tested it, it overflows).

@@ -19,7 +19,7 @@ for lib in $files
do
filename=$(basename "$lib")
filename="${filename%.*}"
clang -S -emit-llvm $lib -o $inc_path/$filename.ll
clang-3.8 -S -emit-llvm $lib -o $inc_path/$filename.ll
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have to discuss this change with care.

@@ -11,7 +11,7 @@ MKBIN=mkdir -p bin
ROOT=src

SRCS=$(ROOT)/utils/ArgsOptions.cpp $(ROOT)/utils/ArgsHandler.cpp $(ROOT)/utils/Bootstrap.cpp $(ROOT)/parser/Parser.cpp $(ROOT)/ast/general/ASTInfo.cpp $(ROOT)/ast/expr/Expr.cpp $(ROOT)/ast/expr/ShiftExpr.cpp $(ROOT)/ast/expr/IncrementExpr.cpp $(ROOT)/ast/expr/InputExpr.cpp $(ROOT)/ast/expr/OutputExpr.cpp $(ROOT)/ast/expr/LoopExpr.cpp $(ROOT)/ast/expr/ArithmeticExpr.cpp $(ROOT)/ast/expr/DebugExpr.cpp $(ROOT)/ast/expr/BreakExpr.cpp $(ROOT)/ast/expr/IfExpr.cpp $(ROOT)/ast/expr/FloatExpr.cpp $(ROOT)/main.cpp
CONFIG=`llvm-config --cxxflags --ldflags --system-libs --libs core mcjit native nativecodegen irreader linker`
CONFIG=`llvm-config-3.8 --cxxflags --ldflags --system-libs --libs core mcjit native nativecodegen irreader linker`
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have to discuss this change with care.


// you can overwrite those functions! :)

int number_size(int number) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should return unsigned int.

}

void b_show_tape(int idx, int *cells, int size) {
// TODO: ellipsize values based on index
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is indeed a good idea! 😄

@luizperes
Copy link
Member

@haskellcamargo could you make the updates from clang-3.8 to clang in order to get your PR approved? I have already created this issue here: #46 and will fix it later, thanks! Ah, and I also created a issue for the travis (that should have broken) #45.

After your PR gets approved, the build will broke because of the test files. What should we do? What do you think @rafaelcn

@rafaelcn
Copy link
Collaborator

We have just to add new .cmp files as suggested in the Hangouts call.

@luizperes
Copy link
Member

Yes, I've done it already @rafaelcn haskellcamargo#1, just waiting for him to approve it

Merging new version, fixing tests and improving code
@luizperes luizperes merged commit 2c38860 into brain-labs:dev Jul 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants