Re: LLVM m4 macros

[lyh.kernel]

Hello Rik,

2013/7/22 Rik <address@hidden>

7/21/13

LYH,

I noticed that in your most recent changeset you used AC_MSG_ERROR("...").
The quote character for m4 is '[' and ']' so I changed that in in this
changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/8d84dc5f5b5d).
Programming in m4 is a pain in the ass...

That's fine. Programming in m4 is new to me. Thanks for your correction.

 

I had a question, though, about using AC_MSG_ERROR.  That construct will
stop configure immediately and abort.  We only use it when something
absolutely has to be present for Octave to compile, such as a BLAS
library.  In this case, if someone has a screwed up installation of LLVM I
think it makes sense just to warn them, or possibly disable LLVM
compilation, but not to prevent Octave itself from compiling.  Thus, I
would avoid the AC_MSG_ERROR macro.

In general the m4 macros should look like this in pseudo-code

check for feature XXX
if (XXX present)
 define HAVE_XXX in config.h
endif

The new tests for LLVM, however, are

check for feature XXX
if (XXX present)
 define HAVE_XXX in config.h
else
  if (! XXX present)
    AC_MSG_ERROR
  endif
endif

As a specific example, I would take the OCTAVE_LLVM_DATALAYOUT_HEADER macro

dnl
dnl Detect TargetData.h or DataLayout.h.
dnl
AC_DEFUN([OCTAVE_LLVM_DATALAYOUT_HEADER], [
  AC_CHECK_HEADER([llvm/DataLayout.h], [
    octave_is_datalayout_header=yes], [
    AC_CHECK_HEADER([llvm/Target/TargetData.h], [
      octave_is_datalayout_header=no], [
      AC_MSG_ERROR([DataLayout.h or Target/TargetData.h is required.])
    ])
  ])
  if test $octave_is_datalayout_header = yes; then
    AC_DEFINE(HAVE_DATALAYOUT, 1,
      [Define to 1 if DataLayout.h exist.])
  fi
])

and simplify it to

dnl
dnl Detect TargetData.h or DataLayout.h.
dnl
AC_DEFUN([OCTAVE_LLVM_DATALAYOUT_HEADER], [
  AC_CHECK_HEADER([llvm/DataLayout.h], [
    octave_is_datalayout_header=yes], [
    octave_is_datalayout_header=no]
    )
  if test $octave_is_datalayout_header = yes; then
    AC_DEFINE(HAVE_DATALAYOUT, 1,
      [Define to 1 if DataLayout.h exist.])
  fi
])

Is this sort of change acceptable?

There is something different. If you enable JIT, there must exist either llvm/DataLayout.h or llvm/Target/TargetData.h. Without both you would get error during compilation. That's why I use AC_MSG_ERROR to stop configuration. I describe my logic in pseudo-code:

If llvm-config exist
    enable JIT
    if llvm/DataLayout.h exist
        Define some macro
    else if llvm/Target/TargetData.h
        Define some macro
    else
        ERROR
    endif
else
    disable JIT
endif

I could disable JIT when I detect neither llvm/DataLayout.h nor llvm/Target/TargetData.h exist, rather than stop the configuration. How do you think?

 

Cheers,
Rik