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

nvc 1.5.1 #77261

Closed
wants to merge 3 commits into from
Closed

nvc 1.5.1 #77261

wants to merge 3 commits into from

Conversation

chenrui333
Copy link
Member

Created with brew bump-formula-pr.

resource blocks may require updates.

@BrewTestBot BrewTestBot added the bump-formula-pr PR was created using `brew bump-formula-pr` label May 14, 2021
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

I think we can switch this from llvm@11 to llvm. Don't forget the llvm@11 reference in the install method.

@carlocab
Copy link
Member

Nope, failed to build. Upstream CI uses the llvm formula though.

Maybe we also need the --enable-vhpi flag?

@nandahkrishna
Copy link
Member

Still fails, checked locally as well.

@carlocab
Copy link
Member

Very weird. Can we try running autogen unconditionally? We'll need to move the deps out of the head block though.

@nandahkrishna
Copy link
Member

nandahkrishna commented May 14, 2021

That results in the same error 😅:

==> ./autogen.sh
==> ./tools/fetch-ieee.sh
==> ./configure --with-llvm=/usr/local/opt/llvm/bin/llvm-config --prefix=/usr/local/Cellar/nvc/1.5.1 --with-system-cc=/usr/bin/clang --enable-vhpi
==> make
Last 15 lines from /Users/<username>/Library/Logs/Homebrew/nvc/04.make:
  AR       lib/libfst.a
  CXXLD    bin/nvc
Undefined symbols for architecture x86_64:
  "_LLVMInitializeX86AsmPrinter", referenced from:
      _cgen in libcgen.a(lib_libcgen_a-cgen.o)
  "_LLVMInitializeX86Target", referenced from:
      _cgen in libcgen.a(lib_libcgen_a-cgen.o)
  "_LLVMInitializeX86TargetInfo", referenced from:
      _cgen in libcgen.a(lib_libcgen_a-cgen.o)
  "_LLVMInitializeX86TargetMC", referenced from:
      _cgen in libcgen.a(lib_libcgen_a-cgen.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [bin/nvc] Error 1
make: *** [all] Error 2

@nandahkrishna
Copy link
Member

The --HEAD build works fine and passes all tests. I think nickg/nvc@251e18d is required.

@carlocab
Copy link
Member

I've gotten the tarball to build with llvm 12. Issue may not be that. Just poking at it some more.

@carlocab
Copy link
Member

Tried building outside of brew. Encountered the same error as in this PR. Then, I tried applying the change at #76646 (just do ln -s $(brew --prefix llvm)/lib/libLLVM.dylib $(brew --prefix llvm)/lib/libLLVM-12.dylib). This allows me to build successfully outside of brew.

When I try to build with brew using a patched llvm as in #76646, the build fails, except with a different error, which shows up later in the build:

  NVC      lib/ieee/IEEE.STD_LOGIC_MISC
ld: file too small (length=0) file '/private/tmp/nvc-20210514-18670-17vg8it/nvc-1.5.1/lib/nvc/_NVC.ENV-body.o' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
** Fatal: /usr/bin/clang failed with status 1
make[1]: *** [lib/nvc/NVC.ENV] Error 1

Seems related to this step

system "./tools/fetch-ieee.sh"

as I didn't run it when I tried building outside of brew.

@carlocab
Copy link
Member

Oops, no it's not. Looks like the step I highlighted above doesn't do anything:

❯ ./tools/fetch-ieee.sh
The IEEE standard library sources are now included in the repository for
convenience. They can also be download from the IEEE SA:

  https://standards.ieee.org/content/dam/ieee-standards/standards/web/download/

Note that the license forbids the distribution of modifications.

@carlocab
Copy link
Member

I don't really understand why deparallelizing the build helps here. I've tried several times now to build outside of brew with make -j4 and I can't reproduce the build failure.

@carlocab
Copy link
Member

Let's try a parallel build here. Maybe my computer just sucks.

@nandahkrishna
Copy link
Member

nandahkrishna commented May 15, 2021

Works now 🎉, this should be good to merge?

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@github-actions github-actions bot added the outdated PR was locked due to age label Jul 6, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 6, 2021
@chenrui333 chenrui333 deleted the bump-nvc-1.5.1 branch December 18, 2022 05:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bump-formula-pr PR was created using `brew bump-formula-pr` outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants