[llvm-commits] [Review request] test: Add the new feature "loadable_module"

[Daniel Dunbar]

Hi Takumi,

All of these patches look great, except the Makefile one w.r.t.
reordering the "Hello" build. I'm not sure I like that particular
approach.

Can you apply the other ones, and we can move the one about the
"Hello" transform build to a separate thread?

 - Daniel

2010/11/26 NAKAMURA Takumi <geek4civic at gmail.com>:
> Rafael, good evening.
> Thank you to comment!
>
> Two patches attached.
>
> * (updated) 0002-test-Use-SharedLibDir-for-loadable-modules.-On-Cygmi.patch
> * (new) 0006-tools-Makefile-Fix-indentation.patch
>
>
> 2010/11/26 Rafael Espíndola <rafael.espindola at gmail.com>:
>>> * 0001-test-Add-the-new-feature-loadable_module.patch
>>> (application/octet-stream) 2K
>>
>>
>> +# Loadable module
>> +# FIXME: This should be supplied by Makefile or autoconf.
>> +if sys.platform in ['win32', 'cygwin']:
>> +    loadable_module = (config.enable_shared == 1)
>>
>> Why can't you use config.enable_shared in every platform?
>
> I assume other platforms can build loadable modules with ENABLE_PIC=1,
> regardless of enable_shared. (and can run one)
> 4 tests with loadable_module is running validly on ToT.
> (eg. linux-elf, freebsd-elf, darwin) (IIRC, --disable-shared)
>
>
>>> * 0002-test-Use-SharedLibDir-for-loadable-modules.-On-Cygmi.patch
>>> (application/octet-stream) 5K
>>>
>>>  It passes $SharedLibDir to lit.
>>
>> -            'link', 'shlibext', 'ocamlopt', 'llvmdsymutil', 'llvmlibsdir',
>> +            'link', 'ocamlopt', 'llvmdsymutil', 'llvmlibsdir',
>> +            'llvmshlibdir', 'shlibext',
>>             'bugpoint_topts']:
>>
>> Why the reordering?
>
> It was functionally nonsense, and it has conflicted to Duncan's commit for me.
>
> I regenerated updated patch. I expect someone (or I myself) would tidy it up.
>
> --- a/test/lit.cfg
> +++ b/test/lit.cfg
> @@ -128,6 +128,7 @@ for line in open(os.path.join(config.llvm_obj_root, 'test',
> 'site.exp')):
>  config.substitutions.append(('%llvmgcc_only', site_exp['llvmgcc']))
>  for sub in ['llvmgcc', 'llvmgxx', 'emitir', 'compile_cxx', 'compile_c',
>             'link', 'shlibext', 'ocamlopt', 'llvmdsymutil', 'llvmlibsdir',
> +            'llvmshlibdir',
>             'bugpoint_topts']:
>     if sub in ('llvmgcc', 'llvmgxx'):
>         config.substitutions.append(('%' + sub,
>
>
>>> * 0004-tools-Makefile-Enable-building-lto-edis-and-bugpoint.patch
>>> (application/octet-stream) 2K
>>>
>>>  It enables building butpoint-passes, edis and LTO on Cygming.
>>>  bugpoint-passes can be built with LLVM.dll.
>>
>> OK, but please reindent the code in the ifdef.
>
> I will fix it up with a separated patch.
>
>
> Thank you, ...Takumi
>